RFR: 8194759: Support caching class mirror objects
Jiangli Zhou
jiangli.zhou at oracle.com
Mon Feb 19 04:45:54 UTC 2018
Hi Ioi,
Thanks for the comments!
> On Feb 14, 2018, at 4:38 PM, Ioi Lam <ioi.lam at oracle.com> wrote:
>
> Hi Jiangli,
>
> Thanks for implementing this RFE. The design looks clean and the performance numbers are promising.
>
> Here are my comments -- basically a lot of nagging but I think it will improve maintainability of the code.
Totally agree. :-) They are really helpful, thanks!
>
> I haven't looked at the tests yet. Will do that next.
>
>
> java_classes.cpp:
> ================
>
> 714 static void initialize_static_field(fieldDescriptor* fd, Handle mirror, TRAPS) {
> 715 assert(mirror.not_null() && fd->is_static(), "just checking");
> 716 bool is_archived_mirror = oopDesc::is_archive_object(mirror());
> 717 BasicType t = fd->field_type();
> 718
> 719 if (fd->has_initial_value()) {
>
> -> Why did you move "BasicType t = fd->field_type();" up? It's not needed outside of the "if" block.
In one of my revisions of initialize_static_field(), ’t’ was used outside the if block. I’ll move it back.
>
> Also, instead of just relying on the subtle value of oopDesc::is_archive_object(mirror()), I think it's better to explicitly pass in the condition:
>
> static void initialize_static_field(fieldDescriptor* fd, Handle mirror, bool fix_archived_mirror, TRAPS) {
> ...
> case T_OBJECT:
> {
> assert(fd->signature() == vmSymbols::string_signature(),
> "just checking");
> >> if (fix_archived_mirror && oopDesc::is_archive_object(mirror())) {
> // Archive the String field and update the pointer.
> assert(DumpSharedSpaces, "must be CDS dump time");
> oop s = mirror()->obj_field(fd->offset());
> ...
>
> Otherwise, it's confusing whether oopDesc::is_archive_object(mirror()) would return true at run time.
Ok, how about checking DumpSharedSpaces and remove assert(DumpSharedSpaces, …)? That way we don’t need to add additional argument to the function. I was trying to keep the overhead minimal, but making it more explicit probably does avoid confusion.
static void initialize_static_field(fieldDescriptor* fd, Handle mirror, bool fix_archived_mirror, TRAPS) {
…
if (DumpSharedSpaces && oopDesc::is_archive_object(mirror())) {
// Archive the String field and update the pointer.
oop s = mirror()->obj_field(fd->offset())
...
>
> and, can "s" be asserted to be a string object?
I’ll try to add an assert.
>
> also, there's a mixed use of 'archive_mirror' vs 'archived_mirror'. Should we use the same word?
I’ll clean them up.
>
> void java_lang_Class::archive_basic_type_mirror(TRAPS)
>
> -> maybe most of this function should be moved to universe.cpp?
I’d like to keep the code for archiving primitive types in javaClasses.cpp so it can be referenced to java_lang_Class::create_baisc_type_mirror(). Breaking the function into separate parts and moving some into universe.cpp probably is less maintainable.
>
> oop java_lang_Class::archive_mirror(Klass* k, TRAPS)
>
> -> this function is very long. Can it be broken down into several pieces?
I wanted to keep it in similar structor as create_mirror(). I’ll try to see if I can break it into smaller functions.
>
>
> #if INCLUDE_CDS
> void java_lang_Class::serialize(SerializeClosure* f) {
> f->do_u4((u4*)&offsets_computed);
> f->do_u4((u4*)&classRedefinedCount_offset);
> f->do_u4((u4*)&_class_loader_offset);
> f->do_u4((u4*)&_component_mirror_offset);
> f->do_u4((u4*)&_module_offset);
> f->do_u4((u4*)&_init_lock_offset);
> CLASS_INJECTED_FIELDS(SERIALIZE_INJECTED_FIELD_OFFSET);
> }
> #endif
>
> -> there are many of these functions and it's easy to get out of sync, especially when someone adds a new field in the future. We should handle these with macros. I'll send my suggestion in a separate mail.
I actually had the same concern about the potential danger, so placed each serialize() function next to the compute_offsets(). But, I had to admit it is not foolproof. I’ll use your suggestion. Thanks.
>
> BTW, are these necessary for caching class mirror objects? If not, they should be done in a separate RFE.
The serialize() for java.lang.Class is necessary. The others help startup as well. I saw small but measurable startup benefit with theses offsets being cached. I’d like to keep the changes together, since they have been tested as a whole. I can file a different RFE, and reference both RFEs when committing. That probably would help. Thanks for bring this up.
>
> klass.hpp:
> =========
>
> u _shared_class_stats;
>
> Should this be _shared_class_flags? "_stats" implies “statistics"
Ok.
>
> I found the name "_not_archived_has_signer" a bit awkward. Why is this needed anyway? Did your changes affect the handling of signed classes?
Signers() was called when checking if a class was from a signed jar file when deciding if a dictionary entry should be skipped at dump time. Now, the object it operates on is an archived object, which is not ‘fully-operable’ (e.g. the oops' klass pointer points to relocated klass metadata, some fields are cleared, etc), so we can’t call singers() after an object is archived. That’s why I added the new flag for checking. How about change the name to _has_signer_and_not_archived?
>
> Similarly, why is the following block removed?
>
> bool ClassLoaderExt::check(ClassLoaderExt::Context *context,
> const ClassFileStream* stream,
> const int classpath_index) {
> if (stream != NULL) {
> - // Ignore any App classes from signed JAR file during CDS archiving
> - // dumping
> - if (DumpSharedSpaces &&
> - SharedClassUtil::is_classpath_entry_signed(classpath_index) &&
> - classpath_index >= _app_paths_start_index) {
> - tty->print_cr("Preload Warning: Skipping %s from signed JAR",
> - context->class_name());
> - return false;
> - }
The ClassLoaderExt::check was only used for the boot loader after we changed to use the java loaders for loading at dump time. The removed case are no longer called for the boot loader. I took it as an opportunity to also clean this up since I’m changing the check signer related code for dump time. I’ll leave this in and clean it up in separate RFE if you prefer.
>
>
> metaspaceShared.cpp:
> ===================
>
> static Klass* get_relocated_klass(Klass* orig_klass) {
> assert(DumpSharedSpaces, "dump time only");
> if (_new_loc_table != NULL) {
> address* pp = _new_loc_table->get((address)orig_klass);
> assert(pp != NULL, "must be");
> Klass* klass = (Klass*)(*pp);
> assert(klass->is_klass(), "must be");
> return klass;
> }
> return NULL;
> }
>
> -> Why is the "if" check necessary? None of the calls get_relocated_klass() actually checks for a NULL return value.
It’s probably left from one of my earlier revisions. I’ll clean it up.
>
> // Archive basic type mirrors
> java_lang_Class::archive_basic_type_mirror(THREAD);
>
> -> The comment doesn't add any information. It should be removed. Also, the function name should be "archive_basic_type_mirrors”.
Ok.
>
> MetaspaceShared::serialize:
>
> -> I would suggest putting the serialization of all the well known klasses to a separate function.
Ok.
>
> -> One thing I notice is there are 3 ways to say "put something into the archive"
>
> MetaspaceShared::dump_closed_archive_heap_objects
> java_lang_Class::archive_mirror
> StringTable::write_to_archive
>
> -> Maybe we should standardize on a single verb? Maybe "dump" is the best? It's short, where "archive" is longer and it's unclear whether it's a verb or noun. OK, "dump" is theoretically also a noun but we never use it in that sense.
I think standardizing on this is a good idea. As we are still at the beginning (sort of) of archiving heap objects, I’d like to do this when things are a bit more mature. So we have better idea on what to use.
>
> instanceKlass.cpp
> -----------------
>
> void InstanceKlass::do_nonstatic_fields(void f(fieldDescriptor*, Handle, TRAPS), Handle mirror, TRAPS) {
>
> -> Is this new function really necessary? I think you can simply declare your own FieldClosure and put the mirror as a instance field
>
> FROM:
> Klass *k = m->klass();
> InstanceKlass::cast(k)->do_nonstatic_fields(
> &reset_archived_mirror_field, Handle(THREAD, archived_m), CHECK);
>
> TO
> Klass *k = m->klass();
> MyFieldClosure fc(archived_m);
> InstanceKlass::cast(k)->do_nonstatic_fields(&fc, CHECK);
>
> // The restore process is synchronized on the class loader
I’ll take a look to see if the new function can be removed.
>
> -> this comment is not necessarily true. Parallel-capable loaders use a different lock per class.
I can remove the comment. Do you have specific information on which lock parallel-capable loaders use? I can add the additional information ...
Thanks!
Jiangli
>
>
> Thanks
> - Ioi
>
>
>
>
> On 2/8/18 3:59 PM, Jiangli Zhou wrote:
>> Please review the change for caching class mirror objects in of CDS archive. Currently, CDS archive contains java.lang.String objects and constant pool resolved_references arrays with resolved string constants (when archiving java heap object is allowed). This change adds the mirror objects. Please see the description below for details.
>>
>> RFE: https://bugs.openjdk.java.net/browse/JDK-8194759 <https://bugs.openjdk.java.net/browse/JDK-8194759>
>> webrev: http://cr.openjdk.java.net/~jiangli/8194759/webrev.00/ <http://cr.openjdk.java.net/~jiangli/8194759/webrev.00/>
>>
>> Tested with all CDS/AppCDS tests (including the new tests added as part of the webrev), hs tier1 - hs tier6 (some of the tiers are in progress).
>>
>>
>> Goal
>> ====
>> Support caching the klass mirror objects (java.lang.Class instances) for archived classes from built-in class loaders at CDS dump time. Runtime can use the cached mirrors without recreating the objects in the java heap.
>> Classes with cached mirrors are not pre-initialized at dump time. Pre-initialization is not a goal in current scope.
>> Support G1 GC, 64-bit (non-Windows). Requires UseCompressedOops and UseCompressedClassPointers.
>>
>> Archiving Mirror Objects at CDS Dump Time
>> ==================================
>> Mirror objects are copied to the 'open archive' heap regions at CDS dump time after class metadata is relocated to the shared archive spaces. In JDK 11, mirrors for archived classes with the built-in loaders (boot, PlatformClassLoader, AppClassLoader) can be archived. Following class mirrors are not archived:
>> * Archived classes with user defined class loaders
>> * Classes with signers (loaded from signed JAR file)
>> The _java_mirror in the archived Klass is reset to NULL if the mirror object is not archived.
>> If archiving java heap objects cannot be supported at current dumping time (e.g. unsupported GC is used), all archived Klass' _java_mirror handles are cleared and set to NULL.
>>
>> Mirrors for InstanceKlasses and arrayKlasses
>> ————————————————————
>> Mirror objects are scrubbed to remove all unsharable field values, including fields updated due to the execution of class initializer or other java code when writing out to the archive file. The cached mirrors are in the state of "being just created" by java_lang_Class::create_mirror() without the unsharable parts (e.g. class_loader, protection_domain, and module, etc.).
>> The process for archiving mirrors for InstanceKlass and arrayKlass:
>> • Allocate and copy the mirror data to the open archive heap region. The oopDesc::_metadata._klass in the archived mirror is updated to point to the relocated java.lang.Class Klass.
>> • Clear all non-static fields in the archived mirror
>> • For arrayKlass only
>> • Archive the component klass mirror object and update the comp_mirror field
>> • For InstanceKlass only
>> • Reload initial values for all local static final fields. If the initial value is a String object, the String object is archived. For any of the static fields without initial value, the field in the archived mirror is set to 0/NULL.
>> • The init_lock object is not archived and the field in the archived mirror is reset to NULL
>> • Clear the protection_domain field in the archived mirror
>> • Clear the class_loader field in the archived mirror
>> • Clear the module field in the archived mirror
>> • Relocate the _klass reference in the archived mirror to point to the archived Klass
>> • Relocate the _array_klass reference in the archived mirror to point to the archived arrayKlass if not NULL
>> • Update the klass _java_mirror handle to point to the archived mirror
>> • Set _has_raw_archived_mirror flag in the archived class
>>
>> Mirrors for Primitive Types
>> ————————————
>> The mirrors for primitive types are created differently (see java_lang_Class::create_basic_type_mirror). All primitive type mirrors except T_VOID mirror have the _array_klass field initialized to corresponding TypeArrayKlass. The 'module' fields in basic type mirrors point to the java.base Module, which is set by ModuleEntryTable::patch_javabase_entries() when java.base Module is known.
>> At dump time, the primitive type mirrors are copied to the open archive heap region. The _array_klass field in the archived mirror is updated to point to the relocated TypeArrayKlass. The 'module' field is cleared and set to NULL.
>> All primitive type mirrors do not have associated 'Klass' metadata. The roots to primitive type mirrors are maintained by the Universe. When writing out the archive, the pointers to the primitive type mirrors are written into the archive.
>>
>> Restoring Mirror Objects at Runtime
>> ============================
>>
>> Restoring Primitive Type Mirrors
>> ——————————————
>> The archived primitive mirrors are used at runtime without recreating. No restoration is needed for the primitive type mirrors. The pointers to the archived primitive type mirrors are reloaded into Universe during VM initialization. GC Access API (RootAccess<IN_ARCHIVE_ROOT>::oop_store()) is used when installing the primitive type mirrors in Universe. The mirror 'module' field is set by ModuleEntryTable::patch_javabase_entries() when java.base Module becomes known.
>>
>> Restoring InstanceKlass and ArrayKlass Mirrors
>> ——————————————————————
>> Mirror objects for non-primitive types are restored when shared classes are loaded and restored at runtime.
>> Mirror restoration process:
>> • For InstanceKlass mirror only
>> • Create init_lock
>> • Set protection_domain domain in the mirror
>> • Set class_loader filed in the mirror
>> • Create a handle for the mirror and set _java_mirror in the shared klass
>> • Clear the _has_raw_archived_mirror flag in the shared klass
>> • Set the module field in the mirror
>>
>> Mirror Fixup and Module Fixup
>> ——————————————
>> java.lang.Class field offsets (used by JVM) are cached at CDS dump time and loaded from the archive at runtime time. Shared classes loaded before java.lang.Class are not pushed onto the fixup_mirror_list. For those shared classes, the archived mirrors are restored directly when the classes are loaded.
>> The mirrors are added to the fixup_module_list before java.base is defined and patched later when the java.lang.Module for java.base is known.
>>
>> GC Considerations
>> ===============
>> When an archived mirror is installed (becomes 'in use') during shared class loading and restoration at runtime, the archived mirror is accessed via GC access API with barrier. The archived object is denoted with IN_ARCHIVE_ROOT and becomes a special root that handled differently by GC and enqueued in SATB implicitly in G1.
>> Similar to the klass constant pool resolved_references array, the archived mirrors resides in the 'open archive' heap region. The mirror fields may be updated with references pointing to other non-archive heap regions during runtime execution. References within the archived mirrors are scanned and followed by GC.
>> Shared classes from builtin loaders are never unloaded after they become 'in use' (loaded from the archive) at runtime, which keeps all 'used' archived mirrors alive after being installed.
>>
>> Class Redefinitiaon/Restransformation with Archived Mirrors
>> ==============================================
>> In VM_RedefineClasses::redefine_single_class(), the mirror from ‘scratch_class' is not copied to ‘the_class’. After redefinition/retransformation, the original mirror in 'the_class' is still used.
>>
>> During verification of the scratch class, the original mirror (handle) from ‘the_class’ is temporary copied into the ‘scratch_class’ to avoid issues. When verification is done, the ‘scratch_class’ is restored to point to the scratch mirror.
>>
>> Class redefinition/retransformation has no impact on the archived mirror from the original class.
>>
>> Thanks!
>>
>> Jiangli
>
More information about the hotspot-runtime-dev
mailing list