RFR: 8253180: ZGC: Implementation of JEP 376: ZGC: Concurrent Thread-Stack Processing [v4]

[Per Liden]

On Thu, 24 Sep 2020 08:55:03 GMT, Erik Österlund <eosterlund at 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