Review request: 8025096: Move the ChunkManager instances out of the VirtualSpaceLists

[Coleen Phillmore]

Stefan,

Thanks for sending this to hotspot-dev.   This change seems okay but the 
original intent was that the free chunks went along with their virtual 
space list, which manage the list of mmap spaces.   So that's why they 
were contained by VirtualSpaceList.  And ChunkManager was more hidden - 
nobody was supposed to know about it.

Now all the SpaceManagers have an extra pointer, and there are two space 
managers per class loader in the case of compressed class pointers, so 
that could add up to an interesting amount of extra memory, especially 
for 2M anonymous classes.

Since SpaceManager has a back pointer to VirtualSpaceList, you could 
still have ChunkManager belong to VSL and indirectly get them:
     SpaceManager::chunk_manager() { return _vs_list->chunk_manager(); }

Of if you don't like that, you could have:

     SpaceManager::chunk_manager() { return *_mdtype == 
Metaspace::ClassType ? _chunk_manager_class : the other one; }

*The C heap allocation looks a lot cleaner, and the change to 
get_new_chunk, which is the main purpose of this change looks really 
good.   I'm just worried about the extra footprint and there is already 
_vs_list.   Maybe neither is needed and could be fetched by _mdtype, 
which wasn't there when _vs_list was added. Can you adjust your change 
to not add this memory?   Or file a new bug to remove the additional 
footprint after this?

Thanks,
Coleen

On 9/19/2013 3:48 PM, Stefan Karlsson wrote:
> http://cr.openjdk.java.net/~stefank/8025096/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8025096
>
> In the fix for 'JDK-8024547 
> <https://bugs.openjdk.java.net/browse/JDK-8024547>: MaxMetaspaceSize 
> should limit the committed memory used by the metaspaces' we need to 
> make a clearer distinction between when new memory is committed and 
> when memory is allocated inside already committed memory.
>
> Moving the ChunkManager, which handles the free chunk list, out of the 
> VirtualSpaceList, which handles the committed metaspace memory, is a 
> step in that direction.
>
> Changes:
> - Move the ChunkManager instances from VirtualSpaceList to SpaceManager.
> - Move the inc_container_count() into ChunkManager::free_chunks_get. 
> Now the Chunk manager calls dec_container_count when chunks are 
> inserted in the chunk free list and inc_container_count when chunks 
> are removed from the chunk free list.
> - Moved the ChunkManager::chunk_freelist_allocate() call out from the 
> VirtualSpaceList::get_new_chunk()
>
> thanks,
> StefanK