Roland, thanks for looking into the fix!
You are right.
I moved VM_ENTRY_MARK to the beginning of the method [1].
Updated webrev in place.
http://cr.openjdk.java.net/~vlivanov/8057967/webrev.01/
Best regards,
Vladimir Ivanov
[1]
diff --git a/src/share/vm/ci/ciCallSite.cpp b/src/share/vm/ci/ciCallSite.cpp
--- a/src/share/vm/ci/ciCallSite.cpp
+++ b/src/share/vm/ci/ciCallSite.cpp
@@ -55,6 +55,8 @@
// Return the target MethodHandle of this CallSite.
ciKlass* ciCallSite::get_context() {
assert(!is_constant_call_site(), "");
+
+ VM_ENTRY_MARK;
oop call_site_oop = get_oop();
InstanceKlass* ctxk =
MethodHandles::get_call_site_context(call_site_oop);
if (ctxk == NULL) {
@@ -63,7 +65,6 @@
java_lang_invoke_CallSite::set_context_cas(call_site_oop,
def_context_oop, /*expected=*/NULL);
ctxk = MethodHandles::get_call_site_context(call_site_oop);
}
- VM_ENTRY_MARK;
return (CURRENT_ENV->get_metadata(ctxk))->as_klass();
}
On 4/15/15 1:16 PM, Roland Westrelin wrote:
Hi Vladimir,
http://cr.openjdk.java.net/~vlivanov/8057967/webrev.01/hotspot/
In ciCallSite::get_context(), is it safe to manipulate a raw oop the way you do
it (with 2 different oops). Can’t it be moved concurrently by the GC?
Roland.
http://cr.openjdk.java.net/~vlivanov/8057967/webrev.01/jdk/
Best regards,
Vladimir Ivanov
On 4/1/15 11:56 PM, Vladimir Ivanov wrote:
http://cr.openjdk.java.net/~vlivanov/8057967/webrev.00/hotspot/
http://cr.openjdk.java.net/~vlivanov/8057967/webrev.00/jdk/
https://bugs.openjdk.java.net/browse/JDK-8057967
HotSpot JITs inline very aggressively through CallSites. The
optimistically treat CallSite target as constant, but record a nmethod
dependency to invalidate the compiled code once CallSite target changes.
Right now, such dependencies have call site class as a context. This
context is too coarse and it leads to context pollution: if some
CallSite target changes, VM needs to enumerate all nmethods which
depends on call sites of such type.
As performance analysis in the bug report shows, it can sum to
significant amount of work.
While working on the fix, I investigated 3 approaches:
(1) unique context per call site
(2) use CallSite target class
(3) use a class the CallSite instance is linked to
Considering call sites are ubiquitous (e.g. 10,000s on some octane
benchmarks), loading a dedicated class for every call site is an
overkill (even VM anonymous).
CallSite target class
(MethodHandle.form->LambdaForm.vmentry->MemberName.clazz->Class<?>) is
also not satisfactory, since it is a compiled LambdaForm VM anonymous
class, which is heavily shared. It gets context pollution down, but
still the overhead is quite high.
So, I decided to focus on (3) and ended up with a mixture of (2) & (3).
Comparing to other options, the complications of (3) are:
- CallSite can stay unlinked (e.g. CallSite.dynamicInvoker()), so
there should be some default context VM can use
- CallSite instances can be shared and it shouldn't keep the context
class from unloading;
It motivated a scheme where CallSite context is initialized lazily and
can change during lifetime. When CallSite is linked with an indy
instruction, it's context is initialized. Usually, JIT sees CallSite
instances with initialized context (since it reaches them through indy),
but if it's not the case and there's no context yet, JIT sets it to
"default context", which means "use target call site".
I introduced CallSite$DependencyContext, which represents a nmethod
dependency context and points (indirectly) to a Class<?> used as a context.
Context class is referenced through a phantom reference
(sun.misc.Cleaner to simplify cleanup). Though it's impossible to
extract referent using Reference.get(), VM can access it directly by
reading corresponding field. Unlike other types of references, phantom
references aren't cleared automatically. It allows VM to access context
class until cleanup is performed. And cleanup resets the context to
NULL, in addition to invalidating all relevant dependencies.
There are 3 context states a CallSite instance can be in:
(1) NULL: no depedencies
(2) DependencyContext.DEFAULT_CONTEXT: dependencies are stored in
call site target class
(3) DependencyContext for some class: dependencies are stored on the
class DependencyContext instance points to
Every CallSite starts w/o a context (1) and then lazily gets one ((2) or
(3) depending on the situation).
State transitions:
(1->3): When a CallSite w/o a context (1) is linked with some indy
call site, it's owner is recorded as a context (3).
(1->2): When JIT needs to record a dependency on a target of a
CallSite w/o a context(1), it sets the context to DEFAULT_CONTEXT and
uses target class to store the dependency.
(3->1): When context class becomes unreachable, a cleanup hook
invalidates all dependencies on that CallSite and resets the context to
NULL (1).
Only (3->1) requires dependency invalidation, because there are no
depedencies in (1) and (2->1) isn't performed.
(1->3) is done in Java code (CallSite.initContext) and (1->2) is
performed in VM (ciCallSite::get_context()). The updates are performed
by CAS, so there's no need in additional synchronization. Other
operations on VM side are volatile (to play well with Java code) and
performed with Compile_lock held (to avoid races between VM operations).
Some statistics:
Box2D, latest jdk9-dev
- CallSite instances: ~22000
- invalidated nmethods due to CallSite target changes: ~60
- checked call_site_target_value dependencies:
- before the fix: ~1,600,000
- after the fix: ~600
Testing:
- dedicated test which excercises different state transitions
- jdk/java/lang/invoke, hotspot/test/compiler/jsr292, nashorn
Thanks!
Best regards,
Vladimir Ivanov
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
