dependencies.cpp:
in check_call_site_target_value, the changes == NULL case should be checking
that the call site hasn't changed. It should probably look more like this:
klassOop Dependencies::check_call_site_target_value(klassOop ctxk, oop
call_site, CallSiteDepChange* changes) {
assert(call_site->is_a(SystemDictionary::CallSite_klass()), "sanity");
// Same CallSite object but different target? Check this specific call site
// if changes is non-NULL or validate all CallSites
if ((changes == NULL || (call_site == changes->call_site())) &&
(java_lang_invoke_CallSite::target(call_site) !=
changes->method_handle())) {
return ctxk; // assertion failed
}
assert(java_lang_invoke_CallSite::target(call_site) ==
changes->method_handle(), "should still be valid");
return NULL; // assertion still valid
}
The final assert is just a paranoia check that a call site hasn't changed
without the dependencies being checked.
interpreterRuntime.cpp:
Please move the dependence check code into universe with the other dependence
check code. Also add some comments explaining why it's doing what it's doing.
doCall.cpp:
Can you put in a comment explaining that VolatileCallSite is never inlined.
Otherwise it looks good.
tom
On Aug 5, 2011, at 6:32 AM, Christian Thalinger wrote:
> http://cr.openjdk.java.net/~twisti/7071653
>
> 7071653: JSR 292: call site change notification should be pushed not pulled
> Reviewed-by:
>
> Currently every speculatively inlined method handle call site has a
> guard that compares the current target of the CallSite object to the
> inlined one. This per-invocation overhead can be removed if the
> notification is changed from pulled to pushed (i.e. deoptimization).
>
> I had to change the logic in TemplateTable::patch_bytecode to skip
> bytecode quickening for putfield instructions when the put_code
> written to the constant pool cache is zero. This is required so that
> every execution of a putfield to CallSite.target calls out to
> InterpreterRuntime::resolve_get_put to do the deoptimization of
> depending compiled methods.
>
> I also had to change the dependency machinery to understand other
> dependencies than class hierarchy ones. DepChange got the super-type
> of two new dependencies, KlassDepChange and CallSiteDepChange.
>
> Tested with JRuby tests and benchmarks, hand-written testcases, JDK
> tests and vm.mlvm tests.
>
> Here is the speedup for the JRuby fib benchmark (first is JDK 7 b147,
> second with 7071653). Since the CallSite targets don't change during
> the runtime of this benchmark we can see the performance benefit of
> eliminating the guard:
>
> $ jruby --server bench/bench_fib_recursive.rb 5 35
> 0.883000 0.000000 0.883000 ( 0.854000)
> 0.715000 0.000000 0.715000 ( 0.715000)
> 0.712000 0.000000 0.712000 ( 0.712000)
> 0.713000 0.000000 0.713000 ( 0.713000)
> 0.713000 0.000000 0.713000 ( 0.712000)
>
> $ jruby --server bench/bench_fib_recursive.rb 5 35
> 0.772000 0.000000 0.772000 ( 0.742000)
> 0.624000 0.000000 0.624000 ( 0.624000)
> 0.621000 0.000000 0.621000 ( 0.621000)
> 0.622000 0.000000 0.622000 ( 0.622000)
> 0.622000 0.000000 0.622000 ( 0.621000)
>
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
