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

[David Holmes]

Hi Ioi,

Continuing ... just a few minor comments

Thanks,
David
------

hotspot/src/share/vm/memory/filemap.cpp

Nit: printing string literals doesn't need to use %s ie:

+     tty->print("%s", "[");
+     tty->vprint(msg, ap);
+     tty->print_cr("%s", "]");

Should just be:

+     tty->print("[");
+     tty->vprint(msg, ap);
+     tty->print_cr("]");

---
Why do we need memset here:

  140 FileMapInfo::FileMapInfo() {
  141   assert(_current_info == NULL, "must be singleton"); // not 
thread safe
  142   _current_info = this;
  143   memset(this, 0, sizeof(FileMapInfo));

---
I don't quite follow the name related logic here:

224         strcpy(strptr, name);

but strcpy rather than strncpy raises a red-flag.

---
What is the role of this:

  230       EXCEPTION_MARK;

Can new_array post exceptions? If so you need to deal with it else the 
EXCEPTION_MARK will terminate the VM abruptly.

---

hotspot/src/share/vm/memory/metaspaceShared.cpp

  779   // Rewrite and unlink classes.
  780   tty->print_cr("Rewriting and linking classes ...");

Is it linking or unlinking?

---

hotspot/src/share/vm/oops/instanceKlass.hpp

+   // was loaded. For archived classes, this filed is either NULL (for 
system

Typo: filed

+   // needed after afterwards.

Typo: after after

----

  test/testlibrary/com/oracle/java/testlibrary/BuildHelper.java

File has the wrong copyright header.


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