Request for review: 8009778: NPG: ClassMetaspaceSize is used before set in set_ergonomics_flags()

[Yumin Qi]

Harold,

(I am not a reviewer)

*** 1461,1476 ****
       if (UseCompressedKlassPointers) {
         if (ClassMetaspaceSize > KlassEncodingMetaspaceMax) {
           warning("Class metaspace size is too large for UseCompressedKlassPointers");
           FLAG_SET_DEFAULT(UseCompressedKlassPointers, false);
         } else if (FLAG_IS_DEFAULT(ClassMetaspaceSize)) {
!         // 100,000 classes seems like a good size, so 100M assumes around 1K
!         // per klass.   The vtable and oopMap is embedded so we don't have a fixed
!         // size per klass.   Eventually, this will be parameterized because it
!         // would also be useful to determine the optimal size of the
!         // systemDictionary.
!         FLAG_SET_ERGO(uintx, ClassMetaspaceSize, 100*M);
         }
       }
     }
     // Also checks that certain machines are slower with compressed oops
     // in vm_version initialization code.
--- 1470,1480 ----
       if (UseCompressedKlassPointers) {
         if (ClassMetaspaceSize > KlassEncodingMetaspaceMax) {
           warning("Class metaspace size is too large for UseCompressedKlassPointers");
           FLAG_SET_DEFAULT(UseCompressedKlassPointers, false);
         } else if (FLAG_IS_DEFAULT(ClassMetaspaceSize)) {
!         FLAG_SET_ERGO(uintx, ClassMetaspaceSize, class_metaspace_size_for_compressed_ptrs());
         }
       }
     }

    The ClassMetasoaceSize is default (2MB) so we call class_metaspace_size_for_compressed_ptrs(), but in this new function:

+ // Return the eventual class metaspace size if compressed klass ptrs are used.
+ inline uintx class_metaspace_size_for_compressed_ptrs() {
+   // 100,000 classes seems like a good size, so 100M assumes around 1K per
+   // klass.   The vtable and oopMap is embedded so we don't have a fixed size
+   // per klass.   Eventually, this will be parameterized because it would
+   // also be useful to determine the optimal size of the systemDictionary.
+   return FLAG_IS_DEFAULT(ClassMetaspaceSize) ? 100*M : ClassMetaspaceSize;
+ }
+

There seems repeat to check if  ClassMetaspaceSize is default. Could we just do

!         FLAG_SET_ERGO(uintx, ClassMetaspaceSize, (FLAG_IS_DEFAULT(ClassMetaspaceSize) ? 100*M : ClassMetaspaceSize));

After this new function called, ClassMetaspaceSize is set to either 100MB or value from vmoption. Then in max_heap_for_compressed_oops():

Do we need to change the code to use the new function?

Thanks
Yumin


  

On 5/30/2013 11:05 AM, harold seigel wrote:
> Hi,
>
> Please review this fix for bug 8009778.
>
> The fix adds a function, class_metaspace_size_for_compressed_ptrs(), 
> that returns what the value will be for ClassMetaspaceSize if 
> CompressedKlassPointers are used.  This enables the correct value for 
> ClassMetaspaceSize to be used by max_heap_for_compressed_oops() before 
> ClassMetaspaceSize gets set in set_ergonomics_flags().
>
> The fix was tested with JCK lang and vm tests, UTE vm.quick.testlist 
> and vm.mlvm.testlist tests, JPRT, and jtreg tests. Tests were run on 
> Linux and Solaris. The debugger was used to step through the code to 
> ensure that the fix behaved as expected.
>
> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_8009778/ 
> <http://cr.openjdk.java.net/%7Ehseigel/bug_8009778/>
>
> Bug link at http://bugs.sun.com/view_bug.do?bug_id=8009778
>
> JBS bug link at https://jbs.oracle.com/bugs/browse/JDK-8009778
>
> Thanks, Harold

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130530/b11d11c9/attachment.html