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

Coleen Phillimore coleen.phillimore at oracle.com
Fri Aug 1 22:29:22 UTC 2014


Ioi and team,   Here is a partial review.

Can you recode this:

+    // Used by the VM to create the ProtectionDomain for the url.
+    private ProtectionDomain getProtectionDomain(URL url) {
+        CodeSource cs = null;
+        CodeSigner[] signers = null;
+        cs = new CodeSource(url, signers);
+
+        return getProtectionDomain(cs);
+    }
+

As

+    // Used by the VM to create the ProtectionDomain for the url.
+    private ProtectionDomain getProtectionDomain(URL url) {
+        CodeSigner[] signers = null;
+        CodeSource cs = new CodeSource(url, signers);
+
+        return getProtectionDomain(cs);
+    }
+

The variable signers is probably useful for documentation rather than 
passing null.

For hotspot changes, this is partial:

in instanceKlass.hpp how can you reuse the Breakpoints field?

+  // CDS uses the field to record the ClassPathEntry where the class was
+  // was loaded. For archived classes, this filed is either NULL (for system
+  // classes), or points to a ClassPathEntry in the archive. When a class is
+  // being loaded from the archive, the VM uses the ClassPathEntry to restore
+  // the class ProtectionDomain. The ClassPathEntry information is no longer
+  // needed after afterwards.
+  union {
    BreakpointInfo* _breakpoints;          // bpt lists, managed by Method*
+    int shared_class_path_index;
+  } _info;


in arguments.cpp, can you add a general comment to fix_appclasspath() so 
one has to only read this C code once?   What is it fixing it to do?  Is 
there some strstr C runtime function you can call instead of p++, 
memmove, etc?

In dictionary.cpp

+        ResourceMark rm;
+        tty->print_cr("Removed error class: %s", ik->external_name());

This should be under some PrintSharedSpaces flag.

There are a LOT of new flags.  Do we really need all of these flags???

in vmStructs.cpp

Since the breakpoints field is changed, there may be associated changes 
in the serviceability agent.

I haven't code reviewed the new files yet.   Monday.

Thanks,
Coleen

On 7/28/14, 7: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



More information about the hotspot-runtime-dev mailing list