Sure, you could just have cache_jvmti_state() return a boolean to bail
out immediately for is_old.
dl
On 5/28/20 7:23 AM, serguei.spit...@oracle.com wrote:
Hi Dean,
Thank you for looking at this!
Okay. Let me check what cab be done in this direction.
There is no point to cache is_old. The compilation has to bail out if
it is discovered to be true.
Thanks,
Serguei
On 5/28/20 00:59, Dean Long wrote:
This seems OK as long as the memory barriers in the thread state
transitions prevent the C++ compiler from doing something like
reading is_old before reading redefinition_count. I would feel
better if both JVMCI and C1/C2 cached is_old and redefinition_count
at the same time (making sure to be in the _thread_in_vm state), then
bail out based on the cached value of is_old.
dl
On 5/26/20 12:04 AM, serguei.spit...@oracle.com wrote:
On 5/25/20 23:39, serguei.spit...@oracle.com wrote:
Please, review a fix for:
https://bugs.openjdk.java.net/browse/JDK-8245126
Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2020/kitchensink-comp.1/
Summary:
The Kitchensink stress test with the Instrumentation module
enabled does
a lot of class retransformations in parallel with all other
stressing.
It provokes the assert at the compiled code installation time:
assert(!method->is_old()) failed: Should not be installing old
methods
The problem is that the CompileBroker::invoke_compiler_on_method
in C2 version
(non-JVMCI tiered compilation) is missing the check that exists
in the JVMCI
part of implementation:
2148 // Skip redefined methods
2149 if (target_handle->is_old()) {
2150 failure_reason = "redefined method";
2151 retry_message = "not retryable";
2152 compilable = ciEnv::MethodCompilable_never;
2153 } else {
. . .
2168 }
The fix is to add this check.
Sorry, forgot to explain one thing.
Compiler code has a special mechanism to ensure the JVMTI class
redefinition did
not happen while the method was compiled, so all the assumptions
remain correct.
2190 // Cache Jvmti state
2191 ci_env.cache_jvmti_state();
Part of this is a check that the value of
JvmtiExport::redefinition_count() is
cached in ciEnv variable: _jvmti_redefinition_count.
The JvmtiExport::redefinition_count() value change means a class
redefinition
happened which also implies some of methods may become old.
However, the method being compiled can be already old at the point
where the
redefinition counter is cached, so the redefinition counter check
does not help much.
Thanks,
Serguei
Testing:
Ran Kitchensink test with the Instrumentation module enabled in mach5
multiple times for 100 times. Without the fix the test normally fails
a couple of times in 200 runs. It does not fail with the fix anymore.
Will also submit hs tiers1-5.
Thanks,
Serguei
