RFR: 8170395: Metaspace initialization queries the wrong chunk freelist

Thomas Stüfe thomas.stuefe at gmail.com
Mon Nov 28 16:48:11 UTC 2016


Hi Stefan,

This looks good. Some small remarks:

- Metaspace::verify_initialized () : could be made static. For clarity I'd
also either rename it to something like "verify_global_initialization" or
to just roll the code out into its only caller, Metaspace::initialize().

- I never liked the ChunkSizes enum names, because they do not indicate
they are sizes, and now that the encompassing enum name "ChunkSizes" is
gone they are even less clear. Would it be possible to rename the former
enum values to "...Size" for better code clarity, e.g. "MediumChunkSize"
instead of "MediumChunk"?

- Metaspace::get_space_manager(MetadataType mdtype) - asserting for
mdType==Class||NonClassType instead of != MetadaTypeCount could be a bit
clearer.

- SpaceManager::adjust_initial_chunk_size () - could we rename this to a
more generic name like "::next_larger_chunksize" or similar? I also wonder
whether this could be combined somehow with
SpaceManager::calc_chunk_size(), which wants to do something similar
(calculate a fitting chunk size for a given smaller allocation size)


Kind Regards, Thomas


On Mon, Nov 28, 2016 at 2:52 PM, Stefan Karlsson <stefan.karlsson at oracle.com
> wrote:

> Hi all,
>
> Please, review this patch to fix metaspace initialization.
>
> http://cr.openjdk.java.net/~stefank/8170395/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8170395
>
> The fix for JDK-8169931 introduced a new assert to ensure that we always
> try to allocate chunks that are any of the three fixed sizes (specialized,
> small, medium) or a humongous chunk (if it is larger then the medium chunk
> size).
>
> During metaspace initialization an initial metaspace chunk is allocated.
> The size of some of the metaspace instances can be specified on the command
> line. For example:
> java -XX:InitialBootClassLoaderMetaspaceSize=30720 -version
>
> If this size is smaller than the medium chunk size and at the same time
> doesn't match the specialized or small chunk size, then we end up hitting
> the assert mentioned above:
> #
> # Internal Error (/scratch/opt/jprt/T/P1/142848
> .erik/s/hotspot/src/share/vm/memory/metaspace.cpp:2359), pid=31643,
> tid=31646
> # assert(size > free_chunks(MediumIndex)->size()) failed: Not a humongous
> chunk
> #
>
> ========================================================================
>
> The most important part of the fix is this line:
> +  // Adjust to one of the fixed chunk sizes (unless humongous)
> +  const size_t adjusted = adjust_initial_chunk_size(requested);
>
> which ensures that we always request either of a specialized, small,
> medium, or humongous chunk size, even if the requested size is neither of
> these.
>
> Most of the other code is refactoring to unify the non-class metaspace and
> the class metaspace code paths to get rid of some of the existing code
> duplication, bring the chunk size calculation nearer to the the actual
> chunk allocation, and make it easier to write a unit test for the new
> adjust_initial_chunk_size function.
>
> ========================================================================
>
> The patch for JDK-8169931 was backed out with JDK-8170355 and will be
> reintroduced as JDK-8170358 when this patch has been reviewed and pushed.
>
> Testing: jprt, unit test, parts of PIT testing (including CDS tests),
> failing test
>
> Thanks,
> StefanK
>


More information about the hotspot-dev mailing list