Need reviewer: jmap -permstat

[Swamy Venkataramanappa]

Hi Hiroshi,

I will take a look at your changes and get back to you.

Thanks,
-Swamy

Hiroshi Yamauchi wrote:
> Hi,
>
> I'd like to contribute this patch. I need a reviewer who is familiar
> with the Hotspot Serviceability Agent.
>
> The description of the patch is something like:
>
>         Accounting for more memory uses associated with class metadata
>         in PermGen which "jmap -permstat" misses.
>
> In a small test, the "jmap -permstat" command reports only about
> 50-60% of the permgen memory usage (compared to the actual usage of
> permgen based on what the "jmap" command reports). This patch will
> increase the number up to 80-90%.
>
> Martin Buchholz did an initial review for me and he is volunteering to
> be the committer.
>
> Here's the patch which is based on a very recent version of
> http://hg.openjdk.java.net/jdk7/jdk7
>
> --- a/agent/src/share/classes/sun/jvm/hotspot/tools/PermStat.java
> +++ b/agent/src/share/classes/sun/jvm/hotspot/tools/PermStat.java
> @@ -266,45 +266,51 @@
>        out.println();
>     }
>
> +   private static long objectSize(Oop oop) {
> +      return oop == null ? 0L : oop.getObjectSize();
> +   }
> +
> +   // Don't count the shared empty arrays
> +   private static long arraySize(Array arr) {
> +     return arr.getLength() != 0L ? arr.getObjectSize() : 0L;
> +   }
> +
>     private long computeSize(InstanceKlass k) {
>        long size = 0L;
> -      // InstanceKlass object size
> +      // the InstanceKlass object itself
>        size += k.getObjectSize();
>
> -      // add ConstantPool size
> -      size += k.getConstants().getObjectSize();
> +      // Constant pool
> +      ConstantPool cp = k.getConstants();
> +      size += cp.getObjectSize();
> +      size += objectSize(cp.getCache());
> +      size += objectSize(cp.getTags());
>
> -      // add ConstantPoolCache, if any
> -      ConstantPoolCache cpCache = k.getConstants().getCache();
> -      if (cpCache != null) {
> -         size += cpCache.getObjectSize();
> +      // Interfaces
> +      size += arraySize(k.getLocalInterfaces());
> +      size += arraySize(k.getTransitiveInterfaces());
> +
> +      // Inner classes
> +      size += objectSize(k.getInnerClasses());
> +
> +      // Fields
> +      size += objectSize(k.getFields());
> +
> +      // Methods
> +      ObjArray methods = k.getMethods();
> +      int nmethods = (int) methods.getLength();
> +      if (nmethods != 0L) {
> +         size += methods.getObjectSize();
> +         for (int i = 0; i < nmethods; ++i) {
> +            Method m = (Method) methods.getObjAt(i);
> +            size += m.getObjectSize();
> +            size += objectSize(m.getConstMethod());
> +         }
>        }
>
> -      // add interfaces size
> -      ObjArray interfaces = k.getLocalInterfaces();
> -      size +=  (interfaces.getLength() != 0L)? interfaces.getObjectSize() : 0L;
> -      ObjArray transitiveInterfaces = k.getTransitiveInterfaces();
> -      size += (transitiveInterfaces.getLength() != 0L)?
> transitiveInterfaces.getObjectSize() : 0L;
> -
> -      // add inner classes size
> -      TypeArray innerClasses = k.getInnerClasses();
> -      size += innerClasses.getObjectSize();
> -
> -      // add fields size
> -      size += k.getFields().getObjectSize();
> -
> -      // add methods size
> -      ObjArray methods = k.getMethods();
> -      size += (methods.getLength() != 0L)? methods.getObjectSize() : 0L;
> -      TypeArray methodOrdering = k.getMethodOrdering();
> -      size += (methodOrdering.getLength() != 0L)?
> methodOrdering.getObjectSize() : 0;
> -
> -      // add each method's size
> -      int numMethods = (int) methods.getLength();
> -      for (int i = 0; i < numMethods; i++) {
> -         Method m = (Method) methods.getObjAt(i);
> -         size += m.getObjectSize();
> -      }
> +      // MethodOrdering - an int array that records the original
> +      // ordering of methods in the class file
> +      size += arraySize(k.getMethodOrdering());
>
>        return size;
>     }
>
> I'd appreciate it if someone at Sun can sponsor this.
>
> Thanks,
> Hiroshi
>