Markus,
It looks correct in general and the comments are good.
However, it makes it a little bit more complex.
I'd suggest to rename the "_saved_unloading" to something more meaningful.
What about "_iterated_unloading", "_processed_unloading" or
"_posted_unloading" ?
No pressure, just some thinking. :)
Thanks,
Serguei
On 6/23/14 8:02 AM, Markus Grönlund wrote:
Greetings,
Kindly asking for reviews for the following change:
Bug: https://bugs.openjdk.java.net/browse/JDK-8047812
Webrev: http://cr.openjdk.java.net/~mgronlun/8047812/webrev01
Description:
The "8038212: Method::is_valid_method() check has performance regression
impact for stackwalking" - changeset introduced a change in how the
ClassLoaderDataGraph::_unloading list of ClassLoaderData's is purged.
This change to the purging of the CLD's work the same as before for most GC's,
but when using CMS GC, SystemDictionary::do_unloading() is called twice with no
explicit purge call in between. On the second call (post-sweep), we can now get
stale class loader oops delivered as part of the Klass closure callbacks from
the _unloading list. Again, this is because there is no explicit purge call in
between these two entries to SystemDictionary::do_unloading() - and being CMS
and concurrent, it is very hard to accommodate a timely and proper purge call
here.
The first do_unloading call comes after CMS concurrent marking, and the second
comes from a Full GC triggered while sweeping the CMS heap.
This fix ensures the unloading purge mechanism to work correctly also for the CMS
collector, in that only CLDs with non-reclaimed class loader oops will deliver klasses
from the _unloading list. In addition, this will ensure a single "logical" pass
is achieved when iterating the unloading list in-between purges (avoiding the processing
of the same data twice).
This fix is precipitated by nightly testing failures with CMS after the
introduction of 8038212: Method::is_valid_method() check has performance
regression
impact for stackwalking" - for example
"nsk/sysdict/vm/stress/jck12a//sysdictj12a008" which is crashing because of
following up stale klass loader oop's from the ClassLoaderDataGraph::_unloading list.
Thanks
Markus
