# HG changeset patch # User twisti # Date 1314619655 25200 # Node ID b27c72d69fd15432a1201f47bd09c029627eccc4 # Parent 8805f8c1e23efc1611133d6a073c32e2d1f498a7 7083184: JSR 292: don't store context class argument with call site dependencies Reviewed-by: jrose, never diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/ci/ciEnv.cpp --- a/src/share/vm/ci/ciEnv.cpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/ci/ciEnv.cpp Mon Aug 29 05:07:35 2011 -0700 @@ -884,19 +884,31 @@ } // ------------------------------------------------------------------ -// ciEnv::check_for_system_dictionary_modification -// Check for changes to the system dictionary during compilation -// class loads, evolution, breakpoints -void ciEnv::check_for_system_dictionary_modification(ciMethod* target) { +// ciEnv::validate_compile_task_dependencies +// +// Check for changes during compilation (e.g. class loads, evolution, +// breakpoints, call site invalidation). +void ciEnv::validate_compile_task_dependencies(ciMethod* target) { if (failing()) return; // no need for further checks - // Dependencies must be checked when the system dictionary changes. - // If logging is enabled all violated dependences will be recorded in - // the log. In debug mode check dependencies even if the system - // dictionary hasn't changed to verify that no invalid dependencies - // were inserted. Any violated dependences in this case are dumped to - // the tty. + // First, check non-klass dependencies as we might return early and + // not check klass dependencies if the system dictionary + // modification counter hasn't changed (see below). + for (Dependencies::DepStream deps(dependencies()); deps.next(); ) { + if (deps.is_klass_type()) continue; // skip klass dependencies + klassOop witness = deps.check_dependency(); + if (witness != NULL) { + record_failure("invalid non-klass dependency"); + return; + } + } + // Klass dependencies must be checked when the system dictionary + // changes. If logging is enabled all violated dependences will be + // recorded in the log. In debug mode check dependencies even if + // the system dictionary hasn't changed to verify that no invalid + // dependencies were inserted. Any violated dependences in this + // case are dumped to the tty. bool counter_changed = system_dictionary_modification_counter_changed(); bool test_deps = counter_changed; DEBUG_ONLY(test_deps = true); @@ -904,22 +916,21 @@ bool print_failures = false; DEBUG_ONLY(print_failures = !counter_changed); - bool keep_going = (print_failures || xtty != NULL); - - int violated = 0; + int klass_violations = 0; for (Dependencies::DepStream deps(dependencies()); deps.next(); ) { + if (!deps.is_klass_type()) continue; // skip non-klass dependencies klassOop witness = deps.check_dependency(); if (witness != NULL) { - ++violated; + klass_violations++; if (print_failures) deps.print_dependency(witness, /*verbose=*/ true); - // If there's no log and we're not sanity-checking, we're done. - if (!keep_going) break; } + // If there's no log and we're not sanity-checking, we're done. + if (!keep_going) break; } - if (violated != 0) { + if (klass_violations != 0) { assert(counter_changed, "failed dependencies, but counter didn't change"); record_failure("concurrent class loading"); } @@ -978,8 +989,8 @@ // Encode the dependencies now, so we can check them right away. dependencies()->encode_content_bytes(); - // Check for {class loads, evolution, breakpoints} during compilation - check_for_system_dictionary_modification(target); + // Check for {class loads, evolution, breakpoints, ...} during compilation + validate_compile_task_dependencies(target); } methodHandle method(THREAD, target->get_methodOop()); diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/ci/ciEnv.hpp --- a/src/share/vm/ci/ciEnv.hpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/ci/ciEnv.hpp Mon Aug 29 05:07:35 2011 -0700 @@ -247,9 +247,9 @@ // Is this thread currently in the VM state? static bool is_in_vm(); - // Helper routine for determining the validity of a compilation - // with respect to concurrent class loading. - void check_for_system_dictionary_modification(ciMethod* target); + // Helper routine for determining the validity of a compilation with + // respect to method dependencies (e.g. concurrent class loading). + void validate_compile_task_dependencies(ciMethod* target); public: enum { diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/code/dependencies.cpp --- a/src/share/vm/code/dependencies.cpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/code/dependencies.cpp Mon Aug 29 05:07:35 2011 -0700 @@ -113,9 +113,9 @@ assert_common_1(no_finalizable_subclasses, ctxk); } -void Dependencies::assert_call_site_target_value(ciKlass* ctxk, ciCallSite* call_site, ciMethodHandle* method_handle) { - check_ctxk(ctxk); - assert_common_3(call_site_target_value, ctxk, call_site, method_handle); +void Dependencies::assert_call_site_target_value(ciCallSite* call_site, ciMethodHandle* method_handle) { + check_ctxk(call_site->klass()); + assert_common_2(call_site_target_value, call_site, method_handle); } // Helper function. If we are adding a new dep. under ctxk2, @@ -135,7 +135,7 @@ } } -void Dependencies::assert_common_1(Dependencies::DepType dept, ciObject* x) { +void Dependencies::assert_common_1(DepType dept, ciObject* x) { assert(dep_args(dept) == 1, "sanity"); log_dependency(dept, x); GrowableArray* deps = _deps[dept]; @@ -148,21 +148,37 @@ } } -void Dependencies::assert_common_2(Dependencies::DepType dept, - ciKlass* ctxk, ciObject* x) { - assert(dep_context_arg(dept) == 0, "sanity"); +void Dependencies::assert_common_2(DepType dept, + ciObject* x0, ciObject* x1) { assert(dep_args(dept) == 2, "sanity"); - log_dependency(dept, ctxk, x); + log_dependency(dept, x0, x1); GrowableArray* deps = _deps[dept]; // see if the same (or a similar) dep is already recorded - if (note_dep_seen(dept, x)) { - // look in this bucket for redundant assertions - const int stride = 2; - for (int i = deps->length(); (i -= stride) >= 0; ) { - ciObject* x1 = deps->at(i+1); - if (x == x1) { // same subject; check the context - if (maybe_merge_ctxk(deps, i+0, ctxk)) { + bool has_ctxk = has_explicit_context_arg(dept); + if (has_ctxk) { + assert(dep_context_arg(dept) == 0, "sanity"); + if (note_dep_seen(dept, x1)) { + // look in this bucket for redundant assertions + const int stride = 2; + for (int i = deps->length(); (i -= stride) >= 0; ) { + ciObject* y1 = deps->at(i+1); + if (x1 == y1) { // same subject; check the context + if (maybe_merge_ctxk(deps, i+0, x0->as_klass())) { + return; + } + } + } + } + } else { + assert(dep_implicit_context_arg(dept) == 0, "sanity"); + if (note_dep_seen(dept, x0) && note_dep_seen(dept, x1)) { + // look in this bucket for redundant assertions + const int stride = 2; + for (int i = deps->length(); (i -= stride) >= 0; ) { + ciObject* y0 = deps->at(i+0); + ciObject* y1 = deps->at(i+1); + if (x0 == y0 && x1 == y1) { return; } } @@ -170,11 +186,11 @@ } // append the assertion in the correct bucket: - deps->append(ctxk); - deps->append(x); + deps->append(x0); + deps->append(x1); } -void Dependencies::assert_common_3(Dependencies::DepType dept, +void Dependencies::assert_common_3(DepType dept, ciKlass* ctxk, ciObject* x, ciObject* x2) { assert(dep_context_arg(dept) == 0, "sanity"); assert(dep_args(dept) == 3, "sanity"); @@ -361,7 +377,7 @@ 3, // unique_concrete_subtypes_2 ctxk, k1, k2 3, // unique_concrete_methods_2 ctxk, m1, m2 1, // no_finalizable_subclasses ctxk - 3 // call_site_target_value ctxk, call_site, method_handle + 2 // call_site_target_value call_site, method_handle }; const char* Dependencies::dep_name(Dependencies::DepType dept) { @@ -375,10 +391,7 @@ } void Dependencies::check_valid_dependency_type(DepType dept) { - for (int deptv = (int) FIRST_TYPE; deptv < (int) TYPE_LIMIT; deptv++) { - if (dept == ((DepType) deptv)) return; - } - ShouldNotReachHere(); + guarantee(FIRST_TYPE <= dept && dept < TYPE_LIMIT, err_msg("invalid dependency type: %d", (int) dept)); } // for the sake of the compiler log, print out current dependencies: @@ -586,8 +599,7 @@ code_byte -= ctxk_bit; DepType dept = (DepType)code_byte; _type = dept; - guarantee((dept - FIRST_TYPE) < (TYPE_LIMIT - FIRST_TYPE), - "bad dependency type tag"); + Dependencies::check_valid_dependency_type(dept); int stride = _dep_args[dept]; assert(stride == dep_args(dept), "sanity"); int skipj = -1; @@ -615,18 +627,35 @@ klassOop Dependencies::DepStream::context_type() { assert(must_be_in_vm(), "raw oops here"); - int ctxkj = dep_context_arg(_type); // -1 if no context arg - if (ctxkj < 0) { - return NULL; // for example, evol_method - } else { - oop k = recorded_oop_at(_xi[ctxkj]); - if (k != NULL) { // context type was not compressed away + + // Most dependencies have an explicit context type argument. + { + int ctxkj = dep_context_arg(_type); // -1 if no explicit context arg + if (ctxkj >= 0) { + oop k = argument(ctxkj); + if (k != NULL) { // context type was not compressed away + assert(k->is_klass(), "type check"); + return (klassOop) k; + } + // recompute "default" context type + return ctxk_encoded_as_null(_type, argument(ctxkj+1)); + } + } + + // Some dependencies are using the klass of the first object + // argument as implicit context type (e.g. call_site_target_value). + { + int ctxkj = dep_implicit_context_arg(_type); + if (ctxkj >= 0) { + oop k = argument(ctxkj)->klass(); assert(k->is_klass(), "type check"); return (klassOop) k; - } else { // recompute "default" context type - return ctxk_encoded_as_null(_type, recorded_oop_at(_xi[ctxkj+1])); } } + + // And some dependencies don't have a context type at all, + // e.g. evol_method. + return NULL; } /// Checking dependencies: @@ -1409,21 +1438,20 @@ } -klassOop Dependencies::check_call_site_target_value(klassOop ctxk, oop call_site, oop method_handle, CallSiteDepChange* changes) { +klassOop Dependencies::check_call_site_target_value(oop call_site, oop method_handle, CallSiteDepChange* changes) { assert(call_site ->is_a(SystemDictionary::CallSite_klass()), "sanity"); assert(method_handle->is_a(SystemDictionary::MethodHandle_klass()), "sanity"); if (changes == NULL) { // Validate all CallSites if (java_lang_invoke_CallSite::target(call_site) != method_handle) - return ctxk; // assertion failed + return call_site->klass(); // assertion failed } else { // Validate the given CallSite if (call_site == changes->call_site() && java_lang_invoke_CallSite::target(call_site) != changes->method_handle()) { assert(method_handle != changes->method_handle(), "must be"); - return ctxk; // assertion failed + return call_site->klass(); // assertion failed } } - assert(java_lang_invoke_CallSite::target(call_site) == method_handle, "should still be valid"); return NULL; // assertion still valid } @@ -1488,7 +1516,7 @@ klassOop witness = NULL; switch (type()) { case call_site_target_value: - witness = check_call_site_target_value(context_type(), argument(1), argument(2), changes); + witness = check_call_site_target_value(argument(0), argument(1), changes); break; default: witness = NULL; diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/code/dependencies.hpp --- a/src/share/vm/code/dependencies.hpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/code/dependencies.hpp Mon Aug 29 05:07:35 2011 -0700 @@ -166,9 +166,14 @@ LG2_TYPE_LIMIT = 4, // assert(TYPE_LIMIT <= (1< recorded_oop_at(argument_index(i)) klassOop context_type(); + bool is_klass_type() { return Dependencies::is_klass_type(type()); } + methodOop method_argument(int i) { oop x = argument(i); assert(x->is_method(), "type"); diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/memory/universe.cpp --- a/src/share/vm/memory/universe.cpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/memory/universe.cpp Mon Aug 29 05:07:35 2011 -0700 @@ -1203,12 +1203,12 @@ // Compute the dependent nmethods that have a reference to a // CallSite object. We use instanceKlass::mark_dependent_nmethod // directly instead of CodeCache::mark_for_deoptimization because we - // want dependents on the class CallSite only not all classes in the - // ContextStream. + // want dependents on the call site class only not all classes in + // the ContextStream. int marked = 0; { MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag); - instanceKlass* call_site_klass = instanceKlass::cast(SystemDictionary::CallSite_klass()); + instanceKlass* call_site_klass = instanceKlass::cast(call_site->klass()); marked = call_site_klass->mark_dependent_nmethods(changes); } if (marked > 0) { diff -r 8805f8c1e23e -r b27c72d69fd1 src/share/vm/opto/callGenerator.cpp --- a/src/share/vm/opto/callGenerator.cpp Sat Aug 27 00:23:47 2011 -0700 +++ b/src/share/vm/opto/callGenerator.cpp Mon Aug 29 05:07:35 2011 -0700 @@ -336,7 +336,7 @@ } CallGenerator* CallGenerator::for_dynamic_call(ciMethod* m) { - assert(m->is_method_handle_invoke(), "for_dynamic_call mismatch"); + assert(m->is_method_handle_invoke() || m->is_method_handle_adapter(), "for_dynamic_call mismatch"); return new DynamicCallGenerator(m); } @@ -715,9 +715,9 @@ // Get an adapter for the MethodHandle. ciMethod* target_method = method_handle->get_method_handle_adapter(); if (target_method != NULL) { - CallGenerator* hit_cg = Compile::current()->call_generator(target_method, -1, false, jvms, true, 1); - if (hit_cg != NULL && hit_cg->is_inline()) - return hit_cg; + CallGenerator* cg = Compile::current()->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS); + if (cg != NULL && cg->is_inline()) + return cg; } } else if (method_handle->Opcode() == Op_Phi && method_handle->req() == 3 && method_handle->in(1)->Opcode() == Op_ConP && method_handle->in(2)->Opcode() == Op_ConP) { @@ -754,13 +754,13 @@ ciMethod* target_method = method_handle->get_invokedynamic_adapter(); if (target_method != NULL) { Compile *C = Compile::current(); - CallGenerator* hit_cg = C->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS); - if (hit_cg != NULL && hit_cg->is_inline()) { + CallGenerator* cg = C->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS); + if (cg != NULL && cg->is_inline()) { // Add a dependence for invalidation of the optimization. if (call_site->is_mutable_call_site()) { - C->dependencies()->assert_call_site_target_value(C->env()->CallSite_klass(), call_site, method_handle); + C->dependencies()->assert_call_site_target_value(call_site, method_handle); } - return hit_cg; + return cg; } } return NULL;