7083184: JSR 292: don't store context class argument with call site dependencies
Reviewed-by: jrose, never
--- a/hotspot/src/share/vm/ci/ciEnv.cpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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());
--- a/hotspot/src/share/vm/ci/ciEnv.hpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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 {
--- a/hotspot/src/share/vm/code/dependencies.cpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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<ciObject*>* 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<ciObject*>* 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;
--- a/hotspot/src/share/vm/code/dependencies.hpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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<<LG2_TYPE_LIMIT))
// handy categorizations of dependency types:
- all_types = ((1<<TYPE_LIMIT)-1) & ((-1)<<FIRST_TYPE),
- non_ctxk_types = (1<<evol_method),
- ctxk_types = all_types & ~non_ctxk_types,
+ all_types = ((1 << TYPE_LIMIT) - 1) & ((-1) << FIRST_TYPE),
+
+ non_klass_types = (1 << call_site_target_value),
+ klass_types = all_types & ~non_klass_types,
+
+ non_ctxk_types = (1 << evol_method),
+ implicit_ctxk_types = (1 << call_site_target_value),
+ explicit_ctxk_types = all_types & ~(non_ctxk_types | implicit_ctxk_types),
max_arg_count = 3, // current maximum number of arguments (incl. ctxk)
@@ -184,9 +189,15 @@
static const char* dep_name(DepType dept);
static int dep_args(DepType dept);
- static int dep_context_arg(DepType dept) {
- return dept_in_mask(dept, ctxk_types)? 0: -1;
- }
+
+ static bool is_klass_type( DepType dept) { return dept_in_mask(dept, klass_types ); }
+
+ static bool has_explicit_context_arg(DepType dept) { return dept_in_mask(dept, explicit_ctxk_types); }
+ static bool has_implicit_context_arg(DepType dept) { return dept_in_mask(dept, implicit_ctxk_types); }
+
+ static int dep_context_arg(DepType dept) { return has_explicit_context_arg(dept) ? 0 : -1; }
+ static int dep_implicit_context_arg(DepType dept) { return has_implicit_context_arg(dept) ? 0 : -1; }
+
static void check_valid_dependency_type(DepType dept);
private:
@@ -250,8 +261,8 @@
}
void assert_common_1(DepType dept, ciObject* x);
- void assert_common_2(DepType dept, ciKlass* ctxk, ciObject* x);
- void assert_common_3(DepType dept, ciKlass* ctxk, ciObject* x, ciObject* x2);
+ void assert_common_2(DepType dept, ciObject* x0, ciObject* x1);
+ void assert_common_3(DepType dept, ciKlass* ctxk, ciObject* x1, ciObject* x2);
public:
// Adding assertions to a new dependency set at compile time:
@@ -264,7 +275,7 @@
void assert_abstract_with_exclusive_concrete_subtypes(ciKlass* ctxk, ciKlass* k1, ciKlass* k2);
void assert_exclusive_concrete_methods(ciKlass* ctxk, ciMethod* m1, ciMethod* m2);
void assert_has_no_finalizable_subclasses(ciKlass* ctxk);
- void assert_call_site_target_value(ciKlass* ctxk, ciCallSite* call_site, ciMethodHandle* method_handle);
+ void assert_call_site_target_value(ciCallSite* call_site, ciMethodHandle* method_handle);
// Define whether a given method or type is concrete.
// These methods define the term "concrete" as used in this module.
@@ -318,7 +329,7 @@
static klassOop check_exclusive_concrete_methods(klassOop ctxk, methodOop m1, methodOop m2,
KlassDepChange* changes = NULL);
static klassOop check_has_no_finalizable_subclasses(klassOop ctxk, KlassDepChange* changes = NULL);
- static klassOop check_call_site_target_value(klassOop ctxk, oop call_site, oop method_handle, CallSiteDepChange* changes = NULL);
+ static klassOop check_call_site_target_value(oop call_site, oop method_handle, CallSiteDepChange* changes = NULL);
// A returned klassOop is NULL if the dependency assertion is still
// valid. A non-NULL klassOop is a 'witness' to the assertion
// failure, a point in the class hierarchy where the assertion has
@@ -455,6 +466,8 @@
oop argument(int i); // => 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");
--- a/hotspot/src/share/vm/memory/universe.cpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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) {
--- a/hotspot/src/share/vm/opto/callGenerator.cpp Sat Aug 27 00:23:47 2011 -0700
+++ b/hotspot/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;