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

[David Holmes]

Hi Ioi,

We seem to have lost core-libs-dev so I added them back.

A couple of minor follow ups.

On 9/08/2014 6:02 PM, Ioi Lam wrote:
> Hi,
>
> Thanks a lot to everyone for the very useful comments. I have updated
> the webrev
>
> Just the delta from the previous round of review:
>
> http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-v3_delta_from_v2/

JDK changes:

URLClassLoader.java:

Doesn't this Note

+      *
+      * NOTE: the logic used here has been duplicated in the VM native code
+      * (search for invocation of definePackageInternal in the HotSpot 
sources).
+      * If this is changed, the VM code also need to be modified.

belong on definePackageInternal, not defineClass ??

---

hotspot changes:

hotspot/src/share/vm/classfile/classLoader.cpp

The scoping of the ResourceMark doesn't look right:

  592   // Iterate over class path entries
  593   for (int start = 0; start < len; start = end) {
  594     while (class_path[end] && class_path[end] != 
os::path_separator()[0]) {
  595       end++;
  596     }
  597     EXCEPTION_MARK;
  598     ResourceMark rm(THREAD);
  599     char* path = NEW_RESOURCE_ARRAY(char, end-start+1);
  600     strncpy(path, &class_path[start], end-start);
  601     path[end-start] = '\0';
  602     update_class_path_entry_list(path, false);
  603 #if INCLUDE_CDS
  604     if (DumpSharedSpaces) {
  605       check_shared_classpath(path);
  606     }
  607 #endif
  608     while (class_path[end] == os::path_separator()[0]) {
  609       end++;
  610     }
  611   }

Doesn't the RESOURCE_ARRAY need to be freed within the ResourceMark block?

---

src/share/vm/runtime/arguments.cpp

3340 // This causes problems with CDS, which requires that all 
directories specified in the classpath
3341 // must be empty.

Should that be "must not be empty"? Or did you mean directory names?

---

src/share/vm/runtime/javaCalls.cpp

+     // may cause undesriable side-effect in the class metadata.

Typo: undesriable; also side-effects

---

David
-----


> All the changes:
>
> http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-v3/
>
> Thanks
> - Ioi
>
> On 7/28/14, 4:09 PM, 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
>