RFR: 8170395: Metaspace initialization queries the wrong chunk freelist

[Mikael Gerdin]

Hi Stefan,

On 2016-11-28 14:52, Stefan Karlsson wrote:
> Hi all,
>
> Please, review this patch to fix metaspace initialization.
>
> http://cr.openjdk.java.net/~stefank/8170395/webrev.01/

Overall I think this change looks good.
One thing I noticed is that the first parameter to
VirtualSpaceList::get_new_chunk
is actually ignored so you might want to just get rid of it, it's just 
confusing to see it. If you decide to do something about get_new_chunk I 
think it wouldn't hurt to have the names of the parameters changed as 
well, "grow_chunks_by_words" is actually "requested_chunk_size" and 
"medium_chunk_bunch" could be something like "suggested_commit_granularity"

You might want to make the "const size_t" constants you moved out of the 
enum to either be "static" (which would be static in the C-sense) or add 
them in an anonymous namespace since otherwise they will pollute the 
global symbol namespace (more so than an enum which is strictly file 
scoped).

The rest of the change looks good to me.

/Mikael

> 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