RFR: 8261090: Store old classfiles in static CDS archive

Ioi Lam iklam at openjdk.java.net
Mon Apr 19 17:15:35 UTC 2021


On Wed, 14 Apr 2021 00:14:46 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> Please review this RFE for storing old classfiles with major version < 50 in static CDS archive.
> During static CDS dump time, old classes won't be verified/rewritten. They will be verified/rewritten during runtime.
> Therefore, the `_constMethod`, `_constants`, and `_cache` of old classes must be stored in the RW region of the archive for runtime rewriting. The `ConstantPool::remove_unshareable_info` will be skipped during dump time and the `ConstantPool::restore_unshareable_info` will be skipped during runtime for old classes.
> 
> Passed tiers 1,2,3,4 tests on mach5.

Looks good to me. Some suggestions for comments and performance improvement.

src/hotspot/share/classfile/verifier.cpp line 289:

> 287:     // which the verifier can't understand.
> 288:     // Bytecodes for shared old classes can be verified because they have
> 289:     // not been rewritten.

Maybe put the above 2 lines under line 290, and change "can be" to "should be"?


// However, bytecodes for shared old classes should be verified because they have
// not been rewritten.

src/hotspot/share/interpreter/rewriter.cpp line 572:

> 570: void Rewriter::rewrite(InstanceKlass* klass, TRAPS) {
> 571:   if (klass->is_shared()) {
> 572:     assert(!klass->is_rewritten(), "rewritten shared classes cannot be rewritten again");

Also add


assert(MetaspaceShared::is_old_class(klass), "only shared old classes aren't rewritten");

src/hotspot/share/oops/constantPool.cpp line 142:

> 140:   it->push(&_tags, MetaspaceClosure::_writable);
> 141:   if (!pool_holder()->is_rewritten()) {
> 142:     it->push(&_cache, MetaspaceClosure::_writable);

This shouldn't be necessary. All ConstantPoolCache are allocated in RW space by default.

src/hotspot/share/oops/klassVtable.cpp line 54:

> 52: 
> 53: bool klassVtable::is_preinitialized_vtable() {
> 54:   return _klass->is_shared() && !MetaspaceShared::remapped_readwrite() && !MetaspaceShared::is_old_class(ik());

Maybe we can avoid calling `MetaspaceShared::is_old_class()` by adding an extra bit into klass.hpp:


#if INCLUDE_CDS
  // Flags of the current shared class.
  u2     _shared_class_flags;
  enum {
    _archived_lambda_proxy_is_available = 2,
    _has_value_based_class_annotation = 4,
+   _is_shared_old_klass = 8
  };
#endif

-------------

Changes requested by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3479


More information about the hotspot-runtime-dev mailing list