RFR JDK-8232222: Set state to 'linked' when an archived class is restored at runtime

[Jiangli Zhou]

Since this change involves security related issues, Ioi, Claes and I
had off-mailing list discussion and initial code review (thanks!).
Thanks Ioi for investigating and providing test cases in security
related area. As the security related parts are settled, I'd like to
resume the discussion and review in the open,  and hope to move this
forward.

My initial change (webrev.00) set all archived classes loaded by
builtin loaders in 'linked' state during restoration in
InstanceKlass::restore_unshareable_info(). Ioi demonstrated a
potential issue for classes loaded by the AppClassLoader (thanks
again). More investigation and work are needed to handle the general
case for AppClassLoader. Following are two proposals provide the
support with narrower scopes with no potential security issue. Option
(1) is an approach that I implemented in one of my earlier patches.

Option (1)
========
Set an archived class in 'linked' state during restoration in
InstanceKlass::restore_unshareable_info() for the following cases:

Case 1): For an archived class loaded by the NULL class loader by
default (when verification is not required for classes loaded by the
NULL class loader, which is the default case).
Case 2): For an archived class loaded by the PlatformClassLoader and
AppClassLoader when verification is disabled at runtime. This use case
(disabling class verification) appears to be important in some cases
for real world applications where verification can be safely disabled.

http://cr.openjdk.java.net/~jiangli/8232222/webrev.01/

Option (2)
========
Set an archived class loaded by the NULL loader in 'linked' state
during runtime restoration in
InstanceKlass::restore_unshareable_info() by default. Option (2) only
supports the case (1) described in option (1).

http://cr.openjdk.java.net/~jiangli/8232222/webrev.02/

Both of the above proposals provide the performance benefit described
in the original review request.

In addition to the performance benefit, my other motivation for this
change is to provide a foundation for potential optimization with more
general class pre-initialization (currently only a few special cases
are supported in the JDK classes) on top of the heap object graph
archiving mechanism (, and constant pool field and method references
pre-resolution) in the future. Apologize for not clearly explain that
in the original review request, although the RFE was linked to the
umbrella RFE for pre-initialization/pre-resolution. I've clarified it
in the RFE as well.

Following are more details on pre-initialization for interested
readers. I'm still investigating this area and flushing out the
details. So the best approach is to do those incrementally. The
information that I provide below may change. It is probably better to
have a general pre-initialization and pre-resolving discussion that's
separate from this review request. And we can move the current change
forward once it's reviewed and accepted.

One immediate improvement that the current change would enable is
simplifying the existing static field pre-initialization applied to
some of the JDK classes. When an archived class can be set in the
'linked' state during runtime restoration, it makes it possible to
place the class in 'initialized' state at restoration. As loading,
verifying, linking/preparing and initializing orders are mandated by
JVMS, a class can only be placed in 'initialized' after it is already
in 'linked' state. Also, we want to avoid a partially initialized
class state. Currently when static fields for a given JDK class is
pre-initialized at CDS dump time, we need to store the preserved field
values separately from the class mirror object (j.l.Class instance).
At runtime, during running the class' static initializer (<clinit>),
the VM finds and stores the values back to the corresponding static
fields in the mirror.  Following is an example where we pre-initialize
java.lang.module.Configuration. In the code below,
VM.initializeFromArchive() is called to retrieve the preserved value
and store back to the mirror. A check is performed to verify if the
value is restored from archive, otherwise we do runtime
initialization. The complication is due to the separate storage for
the preserved values (stored outside the mirror).

    static {
        // Initialize EMPTY_CONFIGURATION from the archive.
        VM.initializeFromArchive(Configuration.class);

        // Create a new empty Configuration if there is no archived version.
        if (EMPTY_CONFIGURATION == null) {
            EMPTY_CONFIGURATION = new Configuration();
        }
    }

With the proposed change in the current review request, it would
enable us to set Configuration in 'initialized' state during runtime
restoration, and therefore allow us to preserve the static field value
as part of the mirror without violating the mandated ordering
requirement. With that, we can eliminate the
VM.initializeFromArchive() call and the if check in the Java code for
some of the use cases. Once Configuration is fully 'restored' at
runtime, there is no need to execute the <clinit> at all. It would
allow us to apply pre-initialization to more JDK classes if desired,
without complicating the Java code.

When we can properly set an archived class to 'initialized' state at
restore time, it then will enable us to pre-resolve related constant
pool method and field references.

Ioi had some questions for pre-initialization and pre-resolution which
I'm including below with my answers inlined. As I'm still
investigating this area, so most likely they are not final.

- Are you going to do the same thing for the initialized state -- all
classes loaded by boot loader will be automatically initialized?

By default, an archived class loaded by the boot loader will only be
placed in 'linked' state at restoration time when it is suitable. For
'value' object that cannot be placed in 'linked' state at restore time
when archiving supported for that is added in the future, it can be
handled differently and the related class can remain in 'alloced'
state during restoration.

An archived class will only be set to 'initialized' during runtime
restoration if the class can be in fully 'initialized' state with all
static fields pre-initialized and preserved. An archived class can be
'flagged' for that case at dump time. Specific implementation details
need further investigation.

- That certainly doesn't seem feasible, so you will probably need to
put some restrictions. So why aren't you putting in those restrictions
now? Do you know what these restrictions are?

The restriction only applies when we add support for more general
'pre-initialization' and 'value' object archiving. Currently, for the
restore-time 'linked' state implementation in the webrevs, the
restriction does not apply.

- Will they be so restrictive that this will not be practical?

As pre-initialization cannot be applied to all cases, selective and
targeted pre-initialization (following current CDS static field
pre-initializing model) is a more flexible approach that provides both
performance benefits and JVMS/language spec compliant.

Comments are welcome and appreciated!

Best regards,
Jiangli




On Thu, Nov 28, 2019 at 9:33 AM Jiangli Zhou <jianglizhou at google.com> wrote:
>
> Hi,
>
> Please review the following optimization related to archived classes'
> (from builtin loaders) runtime restoration and linking. It is not
> intended for OpenJDK 14. After reviewers review the change, I'll wait
> for 14 fork before pushing.
>
> webrev: http://cr.openjdk.java.net/~jiangli/8232222/webrev.00/
> RFE: https://bugs.openjdk.java.net/browse/JDK-8232222
>
> Motivation and details of the change, which are duplicated in the RFE
> =====================================================
>
> When linking a class, InstanceKlass::link_class_impl() first links all
> super classes and super interfaces of the current class. For the
> current class, it then verifies and rewrites the bytecode, links
> methods, initializes the itable and vtable, and sets the current class
> to 'linked' state.
>
> When loading an archived class at runtime,
> SystemDictionary::load_shared_class makes sure the super types (all
> super classes and super interfaces) in the class hierarchy are loaded
> first. If not, the archived class is not used. The archived class is
> restored when 'loading' from the archive. At the end of the
> restoration, all methods are linked. As bytecode verification and
> rewriting are done at CDS dump time, the runtime does not redo the
> operations for an archived class.
>
> If we make sure the itable and vtable are properly initialized (not
> needed for classes loaded by the NULL class loader) and
> SystemDictionaryShared::check_verification_constraints is performed
> for an archived class during restoration, then the archived class
> (from builtin loaders) is effectively in 'linked' state.
>
> For all archived classes loaded by the builtin loaders, we can safely
> set to 'linked' state at the end of restoration. As a result, we can
> save the work for iterating the super types in
> InstanceKlass::link_class_impl() at runtime.
>
> Performance results
> ================
>
> With both JDK 11 and the latest jdk/jdk, the proposed change saves
> ~1.5M instruction execution when running HelloWorld with the default
> CDS. Please see raw data in the RFE. For applications using more
> archived classes at runtime, larger saving should be experienced.
>
> Testing
> ======
> Tested with all jtreg cds/* tests, which include all appcds tests.
> Submit repo testing passed.
>
> The change has also gone through internal testing with very large
> number of tests (all with default CDS enabled) for more than a month.
>
> Best,
> Jiangli