RFR (XL) 8046070 - Class Data Sharing clean up and refactoring, round #2

Ioi Lam ioi.lam at oracle.com
Sun Aug 3 06:59:57 UTC 2014


David, thanks for the comments. I will fix the code as you suggested.

>  225   int index = 0; // Defined here for portability! Do not move
>
> ??? Do we have a C compiler that can't declare loop variables?
I probably had two loops using the "index" variable. I remember some old 
C++ compilers would be confused if you do

     for (int index=0; ...) {}
for (int index=0; ...) {}

and would complain that "index" was declared twice. In any case, I will 
remove the comment here and move the declaration of "index" into the 
"for" statement.

- Ioi


On 8/2/14, 8:23 PM, David Holmes wrote:
> Hi Ioi,
>
> Sending a partial review as I've been delayed again. :(
>
> A few minor comments:
>
> src/share/vm/classfile/classLoader.hpp
>
> This enum is using inconsistent style for the constants:
>
>  149   enum SomeConstants {
>  150     package_hash_table_size = 31,  // Number of buckets
>  151     MAX_CLASSPATH_INDEX = 0x7fffffff
>  152   };
>
> ---
>
> src/share/vm/classfile/dictionary.cpp
>
>  223 void Dictionary::remove_error_classes() {
>
> Does this mean "remove erroneous classes"? What kinds of errors are we 
> referring to here?
>
>  225   int index = 0; // Defined here for portability! Do not move
>
> ??? Do we have a C compiler that can't declare loop variables?
>
>  231       Klass* e = probe->klass();
>  232       InstanceKlass* ik = InstanceKlass::cast(e);
>
> Can we dispense with the curiously named 'e' local?
>
> ---
>
> src/share/vm/classfile/systemDictionary.cpp
>
>  982     guarantee(!DumpSharedSpaces, "must not create anonymoys 
> classes when dumping");
>
> Typo: anonymoys
>
> 1221     // FIXME: locking comment out of date!
>
> This FIXME needs fixing.
>
>
> SharedClassUtil::is_shared_boot_class should be 
> SharedClassUtil::is_shared_class as it doesn't know whether the passed 
> in class is a "boot" class or not.
>
> David
> ------
>
> On 29/07/2014 9:09 AM, Ioi Lam wrote:
>> Hi Folks,
>>
>> Please review the following clean up and refactoring of the CDS code,
>> for JDK9
>>
>>      http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-v2/
>> https://bugs.openjdk.java.net/browse/JDK-8046070
>>
>> Summary of fix:
>>
>>      Clean up and refactor the Class Data Sharing (CDS) code, including:
>>
>>      + Improve archive integrity checking
>>      + Support bytecode verification during archive dumping time
>>      + Allow the user to configure the CDS class list and archive file.
>>      + Allow future extension of the CDS class loading mechanism.
>>
>> Tests:
>>
>>      JPRT
>>      UTE (vm.runtime.testlist, vm.quick.testlist,
>> vm.parallel_class_loading.testlist)
>>      Various adhoc SQE tests on Aurora
>>      JCK
>>
>> Thanks
>> - Ioi



More information about the hotspot-runtime-dev mailing list