RFR(xs): 8202074: Metaspace: If humongous chunk is added to SpaceManager, previous current chunk may not get retired correctly.

[Thomas Stüfe]

Hi all,

may I have reviews for this fix please:

Bug: https://bugs.openjdk.java.net/browse/JDK-8202074
webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8202074-metaspace-retire-after-humongous-chunks-are-allocated/webrev.00/webrev/src/hotspot/share/memory/metaspace.cpp.udiff.html

When a humongous chunk is added to SpaceManager
(SpaceManager::add_chunk(chunk, make_current=true), we make it the
current chunk.

But we do not correctly retire the old chunk. This leads to the
remainder of the used space in the old chunk being wasted.

The obvious smallest-possible fix would be this:

--- a/src/hotspot/share/memory/metaspace.cpp Mon Apr 16 14:29:27 2018 +0530
+++ b/src/hotspot/share/memory/metaspace.cpp Fri Apr 20 06:55:04 2018 +0200
@@ -3573,6 +3573,7 @@
     // chunk.
     if (make_current) {
       // Set as the current chunk but otherwise treat as a humongous chunk.
+      retire_current_chunk();
       set_current_chunk(new_chunk);
     }
     // Link at head.  The _current_chunk only points to a humongous chunk for

However, I decided to clean the code up a bit and make it more
readable. In SpaceManager::add_chunk(), treatment of the non-humongous
chunks and humongous chunks is almost identical, beside the fact that
non-humongous new chunks were always made current and the argument
flag "make_current" was only honored for humongous chunks.

I changed that. I simplified SpaceManager::add_chunk() by removing the
extra treatment for humongous chunks, instead moving the humongous
chunk related decision up to SpaceManager::grow_and_allocate(),
together with a clear comment. That has the side effect that
SpaceManager::add_chunk(chunk, make_current) now always honors the
make_current flag.

Thank you, Thomas