RFR: 8293979: Resolve JVM_CONSTANT_Class references at CDS dump time

[Coleen Phillimore]

On Mon, 19 Sep 2022 04:33:55 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Some `JVM_CONSTANT_Class` entries are guaranteed to resolve to the same value at both CDS dump time and run time:
> 
> - Classes that are resolved during `vmClasses::resolve_all()`. These classes cannot be replaced by JVMTI agents at run time.
> - Supertypes -- at run time, a class `C` can be loaded from the CDS archive only if all of `C`'s super types are also loaded from the CDS archive. Therefore, we know that a `JVM_CONSTANT_Class` reference to a supertype of `C` must resolved to the same value at both CDS dump time and run time.
> 
> By doing the resolution at dump time, we can speed up run time start-up by a little bit.
> 
> The `ClassPrelinker` class added by this PR will also be used in future REFs for pre-resolving other constant pool entries. The ultimate goal is to resolve `invokedynamic` and `invokehandle` so we can significantly improve the start-up time of features such as Lambda expressions and String concatenation. See [JDK-8293336](https://bugs.openjdk.org/browse/JDK-8293336)

I have questions.

src/hotspot/share/cds/classPrelinker.cpp line 48:

> 46:     InstanceKlass* super = ik->java_super();
> 47:     if (super != NULL) {
> 48:       add_one_vm_class(super);

Do you care about order? Would it be better to add the superclasses before the instanceKlass?

src/hotspot/share/cds/classPrelinker.cpp line 59:

> 57: ClassPrelinker::ClassPrelinker() {
> 58:   assert(_singleton == NULL, "must be");
> 59:   _singleton = this;

I'm not sure why you have this?

src/hotspot/share/cds/classPrelinker.cpp line 116:

> 114: Klass* ClassPrelinker::get_resolved_klass_or_null(ConstantPool* cp, int cp_index) {
> 115:   if (cp->tag_at(cp_index).is_klass()) {
> 116:     CPKlassSlot kslot = cp->klass_slot_at(cp_index);

This is another place that CPKlassSlot leaks out.  It would be nice if these were in constantPool.  Maybe this function belongs in the constant pool.

src/hotspot/share/cds/classPrelinker.cpp line 127:

> 125: }
> 126: 
> 127: bool ClassPrelinker::can_archive_resolved_klass(ConstantPool* cp, int cp_index) {

Can the other can_archive_resolved_klass be just above this one so we know why their names are the same?  Also can_archive_vm_resolved_klass is too closely named.

src/hotspot/share/cds/classPrelinker.cpp line 146:

> 144: 
> 145:   bool first_time;
> 146:   _processed_classes.put_if_absent(ik, &first_time);

I don't see any get functions for this hashtable?

src/hotspot/share/cds/classPrelinker.cpp line 194:

> 192:   CPKlassSlot kslot = cp->klass_slot_at(cp_index);
> 193:   int name_index = kslot.name_index();
> 194:   Symbol* name = cp->symbol_at(name_index);

Isn't this klass_name_at() ?  I think these details should stay in constantPool.cpp.

src/hotspot/share/cds/classPrelinker.cpp line 205:

> 203: 
> 204: #if INCLUDE_CDS_JAVA_HEAP
> 205: void ClassPrelinker::resolve_string(constantPoolHandle cp, int cp_index, TRAPS) {

Is this function only needed in this cpp file? Can you make it static and defined before its caller? Are there other functions where this is true?  Does it need to be a member function of ClassPrelinker?

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

PR: https://git.openjdk.org/jdk/pull/10330