Request for review: JDK-8005602 NPG: classunloading does not happen while CMS GC with -XX:+CMSClassUnloadingEnabled is used

[Mikael Gerdin]

Anyone?

On 2013-03-04 15:44, Mikael Gerdin wrote:
> Hi all,
>
> Please review this change to enable CMS to hand back memory for unloaded
> classes when running in concurrent mode.
>
> Bug link:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005602
> Webrev:
> http://cr.openjdk.java.net/~mgerdin/8005602/webrev.4/
>
>
> The core change in this patch is the inserted call in sweep():
> 6101
> 6102   if (should_unload_classes()) {
> 6103     ClassLoaderDataGraph::purge();
> 6104   }
> 6105
>
> This is needed because even though we claim to perform "class unloading"
> in the final remark phase we can't deallocate the memory for classes
> until after we've performed the sweep phase since the sweeper needs to
> find the size of objects even though they and their class is not
> considered live anymore.
>
> The rest of the changes in concurrentMarkSweepGeneration.cpp only relate
> to logging information about released metaspace memory and cms gen
> occupancy to make it easier to analyze what's happening. An example of
> this new logging output is:
>
> 134.069: [CMS-concurrent-sweep-start]
> 134.073: [CMS-concurrent-sweep: 0.004/0.004 secs] [Times: user=0.00
> sys=0.02, real=0.01 secs]
> 134.164: [CMS-resizing: [CMS: 532K->382K(63488K)], [Metaspace:
> 277487K->133228K(370885K/412976K)] ]
> 134.166: [CMS-concurrent-reset-start]
> 134.179: [CMS-concurrent-reset: 0.014/0.014 secs] [Times: user=0.02
> sys=0.00, real=0.02 secs]
>
> The change in genCollectedHeap.cpp is needed to avoid artificially
> inflating the size of the Metaspace memory, since memory is considered
> "used" until purge() has been called. By calling purge() before
> compute_new_size() (as the other collectors do) we use the correct figures.
>
> The disabled assert in metaspace.cpp is faulty because a thread may be
> allocating classes and expanding the metaspace memory while we are in
> compute_new_size, I've verified that this assert can in fact fail on a
> build without these changes.
> The change in ~SpaceManager is because of lock order restrictions,
> sum_capacity_in_chunks_in_use takes the Metaspace lock and would do it
> out of order with regards to the expand_lock.
>
> I've tested these changes primarily by running the ParallelClassLoading
> tests with a small young gen size to enable regular class unloading cycles.
>
> Thanks
> /Mikael