RFR: 8169931: 8k class metaspace chunks misallocated from 4k chunk freelist

[Thomas Stüfe]

Hi Stefan,

this change looks good!

Small nitpick, there exists already a function returning a pointer to the
free list by chunk index (ChunkManager::free_chunks(index)). You could have
implemented ChunkManager::list_chunk_size() using this function (return
free_chunks(index)->size()) and add your assert to
ChunkManager::free_chunks(index) instead. Or, alternativly, just use
free_chunks(index)->size() directly instead of adding list_chunk_size().

Kind Regards, Thomas


On Tue, Nov 22, 2016 at 3:54 PM, Stefan Karlsson <stefan.karlsson at oracle.com
> wrote:

> Hi all,
>
> Please, review this patch to fix a bug in ChunkManager::list_index():
>  http://cr.openjdk.java.net/~stefank/8169931/webrev.01
>
> There's a great description of the bug in the bug report:
>  https://bugs.openjdk.java.net/browse/JDK-8169931
>
> There are two conceptual parts of the metaspace. The _class_ metaspace,
> and the _non-class_ metaspace. They have different chunk sizes, and while
> querying for the list index of a humongous chunk in the class metaspace,
> the code accidentally matched the size against the MediumChunk size of the
> non-class metaspace.
>
> I've changed the code to not query against the global ChunkSizes enum, but
> rather the values stored inside the ChunkManager instances. Therefore, the
> list_index() function was changed into an instance method.
>
> I've written a unit test that provoked the bug. It's a simplified test
> with vm asserts instead of gtest asserts. The reason is that the
> ChunkManager class is currently located in metaspace.cpp, and is not
> accessible from the gtest unit tests.
>
> Testing: jprt, Kitchensink, parallel class loading tests
>
> Thanks,
> StefanK
>