Improve root set used by hat

[A. Sundararajan]

I agree. Static fields of classes loaded by non-bootstrap loaders should 
not be part of root set.

PS. I am not sure  if this is a bug with hat or with heap dumpers (the 
hotspot built-in dumper and SA's dumper). I don't remember how "roots" 
is filled. May be, these static fields should not be flagged as roots?

-Sundar

Kelly O'Hair wrote:
> CC to the serviceability-dev alias.
>
> ---
>
> I have no objection to this change, anybody else care? Sundar? Alan?
> If not I can file a bug, and we can decide who gets to push the change
> into jdk6, and probably jdk7 too.
>
> -kto
>
> Keith Randall wrote:
>> I'm working on tracing down a ClassLoader leak in a web application 
>> I'm working on (see 
>> http://blogs.sun.com/fkieviet/entry/classloader_leaks_the_dreaded_java 
>> for a description of the kind of leak I'm talking about).  I'm using 
>> hat to find sources of the leak, and I have run into a problem where 
>> hat generates a lot of false positives of a particular form.  When I 
>> list the reference chains from the rootset for a ClassLoader, it 
>> lists lots of chains which start at a static field of a class which 
>> was loaded by that classloader.  This isn't a real leak, however, 
>> because if the ClassLoader is otherwise dead, the class containing 
>> that static field will be GCed.
>>
>> I propose that hat shouldn't consider static fields of classes which 
>> aren't loaded by the root classloader as GC roots.  These roots will 
>> be found during the normal heap walk (classloader -> class -> static 
>> field).  Unless of course the classloader isn't reachable, but that 
>> is exactly when we don't want to consider static fields in these 
>> classes as roots.
>>
>> Does this sound like a reasonable thing to do?  It works for me, just 
>> want to make sure it won't break some other use case for hat.
>>
>> Possible patch (to openjdk6 build b16):
>>
>> --- com/sun/tools/hat/internal/model/Snapshot.java
>> ** 331,336 ****
>> --- 331,345 ----
>>           System.out.println();
>>           for (Root r : roots) {
>>               r.resolve(this);
>> +             if (r.getType() == Root.JAVA_STATIC) {
>> +                 JavaClass clazz = (JavaClass) r.getReferer();
>> +                 if (clazz.getLoader() != nullThing) {
>> +                     // this "root" will be discovered through a 
>> reference
>> +                     // to its classloader, so we don't need to 
>> treat it as
>> +                     // an explicit root.
>> +                     continue;
>> +                 }
>> +             }
>>               JavaHeapObject t = findThing(r.getId());
>>               if (t != null) {
>>                   t.addReferenceFromRoot(r);
>>