On Thu, 24 Sep 2020 08:55:03 GMT, Erik Österlund <eosterl...@openjdk.org> wrote:
>> This PR the implementation of "JEP 376: ZGC: Concurrent Thread-Stack >> Processing" (cf. >> https://openjdk.java.net/jeps/376). >> Basically, this patch modifies the epilog safepoint when returning from a >> frame (supporting interpreter frames, c1, c2, >> and native wrapper frames), to compare the stack pointer against a >> thread-local value. This turns return polls into >> more of a swiss army knife that can be used to poll for safepoints, >> handshakes, but also returns into not yet safe to >> expose frames, denoted by a "stack watermark". ZGC will leave frames (and >> other thread oops) in a state of a mess in >> the GC checkpoint safepoints, rather than processing all threads and their >> stacks. Processing is initialized >> automagically when threads wake up for a safepoint, or get poked by a >> handshake or safepoint. Said initialization >> processes a few (3) frames and other thread oops. The rest - the bulk of the >> frame processing, is deferred until it is >> actually needed. It is needed when a frame is exposed to either 1) execution >> (returns or unwinding due to exception >> handling), or 2) stack walker APIs. A hook is then run to go and finish the >> lazy processing of frames. Mutator and GC >> threads can compete for processing. The processing is therefore performed >> under a per-thread lock. Note that disarming >> of the poll word (that the returns are comparing against) is only performed >> by the thread itself. So sliding the >> watermark up will require one runtime call for a thread to note that nothing >> needs to be done, and then update the poll >> word accordingly. Downgrading the poll word concurrently by other threads >> was simply not worth the complexity it >> brought (and is only possible on TSO machines). So left that one out. > > Erik Österlund has updated the pull request incrementally with one additional > commit since the last revision: > > Review: Albert CR 1 I had also pre-reviewed, but did a final semi-quick review. Looks good overall. Just found a couple of minor nitpicks. src/hotspot/share/gc/z/zBarrierSet.cpp line 86: > 84: if (thread->is_Java_thread()) { > 85: JavaThread* const jt = thread->as_Java_thread(); > 86: StackWatermark* watermark = new ZStackWatermark(jt); Please add a const here. src/hotspot/share/gc/z/zStackWatermark.cpp line 43: > 41: nmethod* const nm = cb->as_nmethod_or_null(); > 42: if (nm != NULL) { > 43: bool result = _bs_nm->nmethod_entry_barrier(nm); Please add a const here. src/hotspot/share/gc/z/zVerify.cpp line 88: > 86: virtual void do_thread(Thread* thread); > 87: > 88: bool verify_fixed() const { return _verify_fixed; } Please turn this into three lines. src/hotspot/share/gc/z/zVerify.cpp line 114: > 112: _last_good(0), > 113: _verifying_bad_frames(false) { > 114: ZStackWatermark* stack_watermark = > StackWatermarkSet::get<ZStackWatermark>(jt, StackWatermarkKind::gc); Please add const here. src/hotspot/share/gc/z/zVerify.cpp line 151: > 149: // last processed frame or not. Before it is reached, we expect > everything to > 150: // be good. After reaching it, we expect everything to be bad. > 151: uintptr_t sp = reinterpret_cast<uintptr_t>(frame.sp()); Please add const here. ------------- Marked as reviewed by pliden (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/296
