Improve root set used by hat

[Kelly O'Hair]

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);
>