Deterministic System.identityHashCode for Trace Debugger. #444
Conversation
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
from
687083c to
c9af12a
Compare
zhelenskiy
requested review from
zhelenskiy and
eupp
and removed request for
zhelenskiy
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
4 times, most recently
from
d0fe5f2 to
f6f8002
Compare
| identityHashCode = System.identityHashCode(obj) | ||
| ) | ||
| // ATTENTION: bizarre and crazy code below (might not work for all JVM implementations) | ||
| UnsafeHolder.UNSAFE.putInt(obj, IDENTITY_HASHCODE_OFFSET, initialIdentityHashCode) |
There was a problem hiding this comment.
You need a CAS here, this put races with GC allocation/monitor inflation etc.
Also, you may want to triple-check it with liliput and header shenanigans: https://wiki.openjdk.org/display/lilliput/Compact+Identity+Hashcode
There was a problem hiding this comment.
I call identityHashCode in line 51, so by line 54, the hash-code of the newly created object must be already calculated and stored inside. So, there would be no need for CAS.
There was a problem hiding this comment.
As @zhelenskiy mentioned, the hash-code computation is enforced just before this line, so the hash code should already be stored.
Given that we currently run this tracker under ManagedStrategy, which controls thread scheduling, only single thread can have access to this object at the time, so no other thread may try to synchronize on this object and set its lock bits.
So it looks like the only possibility is if the concurrent GC will decide to change some marking bits concurrently.
I am not sure what exactly can happen in this case. Although the two concurrent writes (our hash code write and GC mark bits write) are to disjoint bits, they are still going to be rounded off to word-sized writes, and thus interfere with each other. Even worse, I am not sure would it be the case that int-sized write at offset of 1 byte can lead to word-tearing?
On the other hand, we don't observe the bugs on our CI right now.
And this is still an early prototype, and we might still decide to choose some alternative implementation strategy for deterministic hash-codes later.
So we left a comment and TODOs to investigate the problem further in the future.
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
from
f6f8002 to
90aa51d
Compare
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
3 times, most recently
from
bbcf605 to
2eda219
Compare
src/jvm/test/org/jetbrains/kotlinx/lincheck_test/AbstractLincheckTest.kt
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckClassVisitor.kt
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/LincheckClassVisitor.kt
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/strategy/managed/ObjectIdentityHashCodeTracker.kt
Show resolved
Hide resolved
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
7 times, most recently
from
5f07261 to
29d50c3
Compare
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
5 times, most recently
from
7ab9d11 to
50c3f25
Compare
…adapted version of #430.
…code substitution mechanism
zhelenskiy
force-pushed
the
trace-debugger-identity-hash-code
branch
from
50c3f25 to
9fa9d1c
Compare
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TransformationUtils.kt
Outdated
Show resolved
Hide resolved
src/jvm/main/org/jetbrains/kotlinx/lincheck/transformation/TransformationUtils.kt
Outdated
Show resolved
Hide resolved
...brains/kotlinx/lincheck/transformation/transformers/DeterministicInvokeDynamicTransformer.kt
Show resolved
Hide resolved
...brains/kotlinx/lincheck/transformation/transformers/DeterministicInvokeDynamicTransformer.kt
Show resolved
Hide resolved
src/jvm/test/org/jetbrains/kotlinx/lincheck_test/trace_debugger/AbstractNativeCallTest.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
This is an adapted version of #430.