RFR: 8291555: Implement alternative fast-locking scheme [v29]

[Erik Österlund]

On Mon, 27 Mar 2023 18:47:31 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

> > > > > > @rkennke Question about ZGC and LockStack::contains(): how does this work with colored pointers? Don't we have to mask the color bits out somehow when comparing? E.g. using `ZAddress::offset()` ?
> 
> > > 
> 
> > > 
> 
> > > > >
> 
> > > 
> 
> > > 
> 
> > > > >
> 
> > > 
> 
> > > 
> 
> > > > > That would be a question for @fisk and/or @stefank. AFAIK, the color bits should be masked by ZGC barriers _before_ the oops enter the synchronization subsystem. But I kinda suspect that we are somehow triggering a ZGC bug here. Maybe we require barriers when reading oops from the lock-stack too?
> 
> > > 
> 
> > > 
> 
> > > > 
> 
> > > 
> 
> > > 
> 
> > > > Oops that are processed in Thread::oops_do should not have load barriers. Other oops should have load barriers.
> 
> > > 
> 
> > > 
> 
> > > Ok, good. The lockstack is processed in JavaThread::oops_do_no_frames() which is called from Thread::oops_do(). But help me here: I believe ZGC processes this stuff concurrently, right? So there might be a window where the lock-stack oops would be unprocessed. The lock-stack would not go under the stack-watermark machinery. And if some code (like JVMTI deadlock detection pause) inspects the lockstack, it might see invalid oops? Is that a plausible scenario, or am I missing something?
> 
> > 
> 
> > The JVMTI deadlock detection runs in a safepoint, doesn't it? Safepoints call start_processing on all threads in safepoint cleanup for non-GC safepoints. That means the lock stack oops should have been processed when the deadlock detection logic runs in a safepoint.
> 
> 
> 
> There appears to be a single code-path that inspects the lock-stack (and also the usual stack under non-fast-locking) outside of safepoints:
> 
> 
> 
> V  [libjvm.so+0x180abd4]  Threads::owning_thread_from_monitor(ThreadsList*, ObjectMonitor*)+0x54  (threads.cpp:1433)
> 
> V  [libjvm.so+0x17a4bfc]  ObjectSynchronizer::get_lock_owner(ThreadsList*, Handle)+0x9c  (synchronizer.cpp:1109)
> 
> V  [libjvm.so+0x1802db0]  ThreadSnapshot::initialize(ThreadsList*, JavaThread*)+0x270  (threadService.cpp:942)
> 
> V  [libjvm.so+0x1803244]  ThreadDumpResult::add_thread_snapshot(JavaThread*)+0x5c  (threadService.cpp:567)
> 
> V  [libjvm.so+0x12a0f64]  jmm_GetThreadInfo+0x480  (management.cpp:1136)
> 
> j  sun.management.ThreadImpl.getThreadInfo1([JI[Ljava/lang/management/ThreadInfo;)V+0 java.management at 21-internal
> 
> 
> 
> Curiously, this seems to be in JMX code, which is also roughly where the failure happens. I came across this code a couple of times and couldn't really tell if it is safe to do that outside of a safepoint. In doubt I have to assume it is not, and maybe this is the source of the failure? WDYT?

Could be. When not running a handshake or safepoint, you need to call start_processing manually on the target thread, which will ensure the oops are fixed until the next safepoint poll.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/10907#issuecomment-1485752396