Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/hotspot/share/cds/aotConstantPoolResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,13 @@ void AOTConstantPoolResolver::preresolve_field_and_method_cp_entries(JavaThread*
bcs.next();
Bytecodes::Code raw_bc = bcs.raw_code();
switch (raw_bc) {
case Bytecodes::_getstatic:
case Bytecodes::_putstatic:
maybe_resolve_fmi_ref(ik, m, raw_bc, bcs.get_index_u2(), preresolve_list, THREAD);
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION; // just ignore
}
break;
case Bytecodes::_getfield:
// no-fast bytecode
case Bytecodes::_nofast_getfield:
Expand Down Expand Up @@ -266,6 +273,7 @@ void AOTConstantPoolResolver::preresolve_field_and_method_cp_entries(JavaThread*
case Bytecodes::_invokespecial:
case Bytecodes::_invokevirtual:
case Bytecodes::_invokeinterface:
case Bytecodes::_invokestatic:
maybe_resolve_fmi_ref(ik, m, raw_bc, bcs.get_index_u2(), preresolve_list, THREAD);
if (HAS_PENDING_EXCEPTION) {
CLEAR_PENDING_EXCEPTION; // just ignore
Expand Down Expand Up @@ -302,13 +310,31 @@ void AOTConstantPoolResolver::maybe_resolve_fmi_ref(InstanceKlass* ik, Method* m
}

Klass* resolved_klass = cp->klass_ref_at(raw_index, bc, CHECK);
const char* is_static = "";

switch (bc) {
case Bytecodes::_getstatic:
case Bytecodes::_putstatic:
if (!VM_Version::supports_fast_class_init_checks()) {
return; // Do not resolve since interpreter lacks fast clinit barriers support
}
InterpreterRuntime::resolve_get_put(bc, raw_index, mh, cp, false /*initialize_holder*/, CHECK);
is_static = " *** static";
break;

case Bytecodes::_getfield:
case Bytecodes::_putfield:
InterpreterRuntime::resolve_get_put(bc, raw_index, mh, cp, false /*initialize_holder*/, CHECK);
break;

case Bytecodes::_invokestatic:
if (!VM_Version::supports_fast_class_init_checks()) {
return; // Do not resolve since interpreter lacks fast clinit barriers support
}
InterpreterRuntime::cds_resolve_invoke(bc, raw_index, cp, CHECK);
is_static = " *** static";
break;

case Bytecodes::_invokevirtual:
case Bytecodes::_invokespecial:
case Bytecodes::_invokeinterface:
Expand All @@ -328,11 +354,11 @@ void AOTConstantPoolResolver::maybe_resolve_fmi_ref(InstanceKlass* ik, Method* m
bool resolved = cp->is_resolved(raw_index, bc);
Symbol* name = cp->name_ref_at(raw_index, bc);
Symbol* signature = cp->signature_ref_at(raw_index, bc);
log_trace(aot, resolve)("%s %s [%3d] %s -> %s.%s:%s",
log_trace(aot, resolve)("%s %s [%3d] %s -> %s.%s:%s%s",
(resolved ? "Resolved" : "Failed to resolve"),
Bytecodes::name(bc), cp_index, ik->external_name(),
resolved_klass->external_name(),
name->as_C_string(), signature->as_C_string());
name->as_C_string(), signature->as_C_string(), is_static);
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/hotspot/share/cds/classListWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,9 @@ void ClassListWriter::write_resolved_constants_for(InstanceKlass* ik) {
if (field_entries != nullptr) {
for (int i = 0; i < field_entries->length(); i++) {
ResolvedFieldEntry* rfe = field_entries->adr_at(i);
if (rfe->is_resolved(Bytecodes::_getfield) ||
if (rfe->is_resolved(Bytecodes::_getstatic) ||
rfe->is_resolved(Bytecodes::_putstatic) ||
rfe->is_resolved(Bytecodes::_getfield) ||
rfe->is_resolved(Bytecodes::_putfield)) {
list.at_put(rfe->constant_pool_index(), true);
print = true;
Expand All @@ -292,6 +294,7 @@ void ClassListWriter::write_resolved_constants_for(InstanceKlass* ik) {
if (rme->is_resolved(Bytecodes::_invokevirtual) ||
rme->is_resolved(Bytecodes::_invokespecial) ||
rme->is_resolved(Bytecodes::_invokeinterface) ||
rme->is_resolved(Bytecodes::_invokestatic) ||
rme->is_resolved(Bytecodes::_invokehandle)) {
list.at_put(rme->constant_pool_index(), true);
print = true;
Expand Down
4 changes: 3 additions & 1 deletion src/hotspot/share/cds/finalImageRecipes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ void FinalImageRecipes::record_recipes_for_constantpool() {
if (field_entries != nullptr) {
for (int i = 0; i < field_entries->length(); i++) {
ResolvedFieldEntry* rfe = field_entries->adr_at(i);
if (rfe->is_resolved(Bytecodes::_getfield) ||
if (rfe->is_resolved(Bytecodes::_getstatic) ||
rfe->is_resolved(Bytecodes::_putstatic) ||
rfe->is_resolved(Bytecodes::_getfield) ||
rfe->is_resolved(Bytecodes::_putfield)) {
cp_indices.append(rfe->constant_pool_index());
flags |= CP_RESOLVE_FIELD_AND_METHOD;
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/interpreter/interpreterRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ void InterpreterRuntime::cds_resolve_invoke(Bytecodes::Code bytecode, int method
switch (bytecode) {
case Bytecodes::_invokevirtual: LinkResolver::cds_resolve_virtual_call (call_info, link_info, CHECK); break;
case Bytecodes::_invokeinterface: LinkResolver::cds_resolve_interface_call(call_info, link_info, CHECK); break;
case Bytecodes::_invokestatic: LinkResolver::cds_resolve_static_call (call_info, link_info, CHECK); break;
case Bytecodes::_invokespecial: LinkResolver::cds_resolve_special_call (call_info, link_info, CHECK); break;

default: fatal("Unimplemented: %s", Bytecodes::name(bytecode));
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/interpreter/linkResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,10 @@ void LinkResolver::resolve_static_call(CallInfo& result,
JFR_ONLY(Jfr::on_resolution(result, CHECK);)
}

void LinkResolver::cds_resolve_static_call(CallInfo& result, const LinkInfo& link_info, TRAPS) {
resolve_static_call(result, link_info, /*initialize_class*/false, CHECK);
}

// throws linktime exceptions
Method* LinkResolver::linktime_resolve_static_method(const LinkInfo& link_info, TRAPS) {

Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/interpreter/linkResolver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ class LinkResolver: AllStatic {

static void cds_resolve_virtual_call (CallInfo& result, const LinkInfo& link_info, TRAPS);
static void cds_resolve_interface_call(CallInfo& result, const LinkInfo& link_info, TRAPS);
static void cds_resolve_static_call(CallInfo& result, const LinkInfo& link_info, TRAPS);
static void cds_resolve_special_call (CallInfo& result, const LinkInfo& link_info, TRAPS);

// same as above for compile-time resolution; but returns null handle instead of throwing
Expand Down
48 changes: 23 additions & 25 deletions src/hotspot/share/oops/cpCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ void ConstantPoolCache::set_direct_or_vtable_call(Bytecodes::Code invoke_code,
}
if (invoke_code == Bytecodes::_invokestatic) {
assert(method->method_holder()->is_initialized() ||
method->method_holder()->is_reentrant_initialization(JavaThread::current()),
method->method_holder()->is_reentrant_initialization(JavaThread::current()) ||
(CDSConfig::is_dumping_archive() && VM_Version::supports_fast_class_init_checks()),
"invalid class initialization state for invoke_static");

if (!VM_Version::supports_fast_class_init_checks() && method->needs_clinit_barrier()) {
Expand Down Expand Up @@ -428,8 +429,13 @@ void ConstantPoolCache::remove_resolved_field_entries_if_non_deterministic() {
ResolvedFieldEntry* rfi = _resolved_field_entries->adr_at(i);
int cp_index = rfi->constant_pool_index();
bool archived = false;
bool resolved = rfi->is_resolved(Bytecodes::_getfield) ||
rfi->is_resolved(Bytecodes::_putfield);
bool resolved = false;

if (rfi->is_resolved(Bytecodes::_getfield) || rfi->is_resolved(Bytecodes::_putfield) ||
((rfi->is_resolved(Bytecodes::_getstatic) || rfi->is_resolved(Bytecodes::_putstatic)) && VM_Version::supports_fast_class_init_checks())) {
resolved = true;
}

if (resolved && !CDSConfig::is_dumping_preimage_static_archive()
&& AOTConstantPoolResolver::is_resolution_deterministic(src_cp, cp_index)) {
rfi->mark_and_relocate();
Expand All @@ -444,11 +450,12 @@ void ConstantPoolCache::remove_resolved_field_entries_if_non_deterministic() {
Symbol* klass_name = cp->klass_name_at(klass_cp_index);
Symbol* name = cp->uncached_name_ref_at(cp_index);
Symbol* signature = cp->uncached_signature_ref_at(cp_index);
log.print("%s field CP entry [%3d]: %s => %s.%s:%s",
log.print("%s field CP entry [%3d]: %s => %s.%s:%s%s",
(archived ? "archived" : "reverted"),
cp_index,
cp->pool_holder()->name()->as_C_string(),
klass_name->as_C_string(), name->as_C_string(), signature->as_C_string());
klass_name->as_C_string(), name->as_C_string(), signature->as_C_string(),
rfi->is_resolved(Bytecodes::_getstatic) || rfi->is_resolved(Bytecodes::_putstatic) ? " *** static" : "");
}
ArchiveBuilder::alloc_stats()->record_field_cp_entry(archived, resolved && !archived);
}
Expand All @@ -464,10 +471,8 @@ void ConstantPoolCache::remove_resolved_method_entries_if_non_deterministic() {
bool resolved = rme->is_resolved(Bytecodes::_invokevirtual) ||
rme->is_resolved(Bytecodes::_invokespecial) ||
rme->is_resolved(Bytecodes::_invokeinterface) ||
rme->is_resolved(Bytecodes::_invokehandle);

// Just for safety -- this should not happen, but do not archive if we ever see this.
resolved &= !(rme->is_resolved(Bytecodes::_invokestatic));
rme->is_resolved(Bytecodes::_invokehandle) ||
(rme->is_resolved(Bytecodes::_invokestatic) && VM_Version::supports_fast_class_init_checks());

if (resolved && !CDSConfig::is_dumping_preimage_static_archive()
&& can_archive_resolved_method(src_cp, rme)) {
Expand Down Expand Up @@ -495,8 +500,8 @@ void ConstantPoolCache::remove_resolved_method_entries_if_non_deterministic() {
resolved_klass->name()->as_C_string(),
(rme->is_resolved(Bytecodes::_invokestatic) ? " *** static" : ""));
}
ArchiveBuilder::alloc_stats()->record_method_cp_entry(archived, resolved && !archived);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It certainly looks right to move this outside the test for logging being enabled but does it make any practical difference i.e.is the statistic used for anything other than this logging category?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I don't it affects anything other than the stats for logging. I think this was just a bug.

}
ArchiveBuilder::alloc_stats()->record_method_cp_entry(archived, resolved && !archived);
}
}

Expand Down Expand Up @@ -534,6 +539,7 @@ void ConstantPoolCache::remove_resolved_indy_entries_if_non_deterministic() {
}

bool ConstantPoolCache::can_archive_resolved_method(ConstantPool* src_cp, ResolvedMethodEntry* method_entry) {
LogStreamHandle(Trace, aot, resolve) log;
InstanceKlass* pool_holder = constant_pool()->pool_holder();
if (pool_holder->defined_by_other_loaders()) {
// Archiving resolved cp entries for classes from non-builtin loaders
Expand All @@ -554,6 +560,12 @@ bool ConstantPoolCache::can_archive_resolved_method(ConstantPool* src_cp, Resolv
if (method_entry->method()->is_continuation_native_intrinsic()) {
return false; // FIXME: corresponding stub is generated on demand during method resolution (see LinkResolver::resolve_static_call).
}
if (method_entry->is_resolved(Bytecodes::_invokehandle) && !CDSConfig::is_dumping_method_handles()) {
return false;
}
if (method_entry->method()->is_method_handle_intrinsic() && !CDSConfig::is_dumping_method_handles()) {
return false;
}
}

int cp_index = method_entry->constant_pool_index();
Expand All @@ -562,21 +574,7 @@ bool ConstantPoolCache::can_archive_resolved_method(ConstantPool* src_cp, Resolv
if (!AOTConstantPoolResolver::is_resolution_deterministic(src_cp, cp_index)) {
return false;
}

if (method_entry->is_resolved(Bytecodes::_invokeinterface) ||
method_entry->is_resolved(Bytecodes::_invokevirtual) ||
method_entry->is_resolved(Bytecodes::_invokespecial)) {
return true;
} else if (method_entry->is_resolved(Bytecodes::_invokehandle)) {
if (CDSConfig::is_dumping_method_handles()) {
// invokehandle depends on archived MethodType and LambdaForms.
return true;
} else {
return false;
}
} else {
return false;
}
return true;
}
#endif // INCLUDE_CDS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,22 @@
* @run main/othervm -Dcds.app.tester.workflow=DYNAMIC -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:. ResolvedConstants DYNAMIC
*/

/*
* @test id=aot
* @summary Dump time resolution of constant pool entries (AOT workflow).
* @requires vm.cds
* @requires vm.cds.supports.aot.class.linking
* @requires vm.compMode != "Xcomp"
* @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds/test-classes/
* @build OldProvider OldClass OldConsumer StringConcatTestOld
* @build ResolvedConstants
* @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar
* ResolvedConstantsApp ResolvedConstantsFoo ResolvedConstantsBar
* MyInterface InterfaceWithClinit NormalClass
* OldProvider OldClass OldConsumer SubOfOldClass
* StringConcatTest StringConcatTestOld
* @run driver ResolvedConstants AOT --two-step-training
*/
import java.util.function.Consumer;
import jdk.test.lib.cds.CDSOptions;
import jdk.test.lib.cds.CDSTestUtils;
Expand Down Expand Up @@ -122,12 +138,15 @@ static void checkAssemblyOutput(String args[], OutputAnalyzer out) {
.shouldMatch(ALWAYS("field.* ResolvedConstantsBar => ResolvedConstantsBar.a:I"))
.shouldMatch(ALWAYS("field.* ResolvedConstantsBar => ResolvedConstantsFoo.a:I"))
.shouldMatch(ALWAYS("field.* ResolvedConstantsFoo => ResolvedConstantsFoo.a:I"))
.shouldMatch(ALWAYS("field.* ResolvedConstantsApp => ResolvedConstantsApp.static_i:I"))

// Resolve field references to child classes ONLY when using -XX:+AOTClassLinking
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsFoo => ResolvedConstantsBar.static_b:I"))
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsFoo => ResolvedConstantsBar.a:I"))
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsFoo => ResolvedConstantsBar.b:I"))

// Resolve field references to unrelated classes ONLY when using -XX:+AOTClassLinking
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsApp => ResolvedConstantsBar.static_b:I"))
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsApp => ResolvedConstantsBar.a:I"))
.shouldMatch(AOTLINK_ONLY("field.* ResolvedConstantsApp => ResolvedConstantsBar.b:I"));

Expand All @@ -150,8 +169,8 @@ static void checkAssemblyOutput(String args[], OutputAnalyzer out) {
.shouldMatch(ALWAYS("method.*: ResolvedConstantsApp ResolvedConstantsApp.privateInstanceCall:"))
.shouldMatch(ALWAYS("method.*: ResolvedConstantsApp ResolvedConstantsApp.publicInstanceCall:"))

// Should not resolve references to static method
.shouldNotMatch(ALWAYS("method.*: ResolvedConstantsApp ResolvedConstantsApp.staticCall:"))
// Should resolve references to static method
.shouldMatch(ALWAYS("method.*: ResolvedConstantsApp ResolvedConstantsApp.staticCall:"))

// Should resolve references to method in super type
.shouldMatch(ALWAYS("method.*: ResolvedConstantsBar ResolvedConstantsFoo.doBar:"))
Expand All @@ -164,7 +183,12 @@ static void checkAssemblyOutput(String args[], OutputAnalyzer out) {
.shouldMatch(AOTLINK_ONLY("method.*: ResolvedConstantsApp java/io/PrintStream.println:"))
.shouldMatch(AOTLINK_ONLY("method.*: ResolvedConstantsBar java/lang/Class.getName:"))

// Resole resolve methods in unrelated classes ONLY when using -XX:+AOTClassLinking
// Resolve method references to child classes ONLY when using -XX:+AOTClassLinking
.shouldMatch(AOTLINK_ONLY("method.* ResolvedConstantsFoo ResolvedConstantsBar.static_doit"))
.shouldMatch(AOTLINK_ONLY("method.* ResolvedConstantsFoo ResolvedConstantsBar.doit2"))

// Resolve methods in unrelated classes ONLY when using -XX:+AOTClassLinking
.shouldMatch(AOTLINK_ONLY("method.*: ResolvedConstantsApp ResolvedConstantsBar.static_doit:"))
.shouldMatch(AOTLINK_ONLY("method.*: ResolvedConstantsApp ResolvedConstantsBar.doit:"))

// End ---
Expand Down Expand Up @@ -203,11 +227,14 @@ public static void main(String args[]) {
System.out.println("Hello ResolvedConstantsApp");
ResolvedConstantsApp app = new ResolvedConstantsApp();
ResolvedConstantsApp.staticCall();
ResolvedConstantsApp.static_i ++;
app.privateInstanceCall();
app.publicInstanceCall();
Object a = app;
((Runnable)a).run();

ResolvedConstantsBar.static_b += 10;
ResolvedConstantsBar.static_doit();
ResolvedConstantsFoo foo = new ResolvedConstantsFoo();
ResolvedConstantsBar bar = new ResolvedConstantsBar();
bar.a ++;
Expand All @@ -218,6 +245,7 @@ public static void main(String args[]) {
StringConcatTest.test();
StringConcatTestOld.main(null);
}
private static int static_i = 10;
private static void staticCall() {}
private void privateInstanceCall() {}
public void publicInstanceCall() {}
Expand Down Expand Up @@ -313,13 +341,21 @@ void doit() {
}

void doBar(ResolvedConstantsBar bar) {
ResolvedConstantsBar.static_b += 1;
ResolvedConstantsBar.static_doit();

bar.a ++;
bar.b ++;
bar.doit2();
}
}

class ResolvedConstantsBar extends ResolvedConstantsFoo {
public static int static_b = 10;
int b = 2;
public static void static_doit() {
}

void doit() {
System.out.println("Hello ResolvedConstantsBar and " + ResolvedConstantsFoo.class.getName());
System.out.println("a = " + a);
Expand All @@ -330,4 +366,8 @@ void doit() {

((ResolvedConstantsFoo)this).doBar(this);
}

void doit2() {

}
}