RFR (M): 8003420: NPG: make new GC root for pd_set

[Thomas Schatzl]

Hi all,

  I tried to incorporate all suggestions so far, here is a list of the
changes:

- fixing the capitalization of the first letter of the comments
- added Ioi's comment about the protection domain table
- implemented Coleen's suggestions to remove the reference counting, and
use an unlink-pass after class unloading. Class unloading (i.e. in the
SystemDictionary::do_unloading() method) is the only case where we need
to unlink, as in all other cases we use the whole contents of the PD
cache table as roots anyway.
All collectors call SystemDictionary::do_unloading().
- added a newline after the printout of the PD cache table
- removed the ProtectionDomainCacheSize global as the sizes have been
tested to be appropriate, and automatic sizing will be implemented
later.
- added missing ProtectionDomainCacheEntry Java source file for the
agent to the webrev
- tried to group methods "naturally"
- fixed a few "ProctectionDomain*" typos

(note that globals.hpp and arguments.cpp files have no change in the
current revision. Webrev apparently picked them up because in earlier
revisions there has been some).

Thanks everyone, looks much better now, especially removing the
reference counting.

Bugs.openjdk.java.net
https://bugs.openjdk.java.net/browse/JDK-8003420

Webrev:
http://cr.openjdk.java.net/~tschatzl/8003420/webrev.1/

Testing:

jprt, manual verification of all operations (add, unlink), dacapo/scala
benchmarks with VerifyBefore/AfterGC; have some Aurora tests for the
vm.parallel_class_loading.testlist vm.quick.testlist vm.runtime.testlist
in the queue.

Some additional comments inlined.

On Thu, 2013-09-26 at 09:32 -0700, Coleen Phillmore wrote:
> Some clarifications.   Thanks Thomas!
> 
> On 9/26/2013 8:53 AM, Thomas Schatzl wrote:
> > Hi Coleen,
> >
> > On Thu, 2013-09-26 at 08:03 -0700, Coleen Phillmore wrote:
> >> Hi Thomas,
> >> Some additional minor comments:
>
> The likelihood of bad decisions seems rather low, because this is a 
> relatively small hash table so unless our performance team tested a 
> larger value (they or related groups using FMW also use 
> PredictedLoadedClassCount), I would not expose this value.   If they did 
> use it, use some ratio based on PredictedLoadedClassCount.

That's true, the tested values were fine. For the sake of simplicity I
removed the global and use the default values during instantiation.

> >> Also, I will double check that classes on the boot class loader never
> >> have protection domain sets.   I believe that's the case.
> > Ioi had a test case that showed that, although I did not have time to
> > check that.
> 
> I believe it.

I tried Ioi's test program, but did not get any successful
is_strongly_reachable() check in always_strong_oops_do(). Ioi, is there
something special I need to do (command line?)?

I kept the existing code for always_strong_oops_do(). Any optimizations
for finding the PDs quickly when class unloading (during
always_strong_oops_do) cannot come quick enough :)  We are working on
class unloading during concurrent marking for g1, and this will be an
issue.

> >> is_strongly_reachable in Dictionary also checks !ClassUnloading but
> >> checking that can be hoisted above the loop.
> > Thanks for your review. I will start working on these issues and will
> > send another webrev as they are finished.

Thanks,
Thomas