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

[Stefan Karlsson]

On Tue, 22 Sep 2020 11:43:41 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.

Partially re-reviewed the changes.

src/hotspot/cpu/x86/templateInterpreterGenerator_x86_64.cpp line 194:

> 192:
> 193:     Label slow_path;
> 194:     __ safepoint_poll(slow_path, r15_thread, true /* at_return */, false /* in_nmethod */);

Why is this tagged with 'true /* at_return */? Same for line 240. The _x86_32 version uses false.

src/hotspot/share/c1/c1_Runtime1.cpp line 515:

> 513:   if (thread->last_frame().is_runtime_frame()) {
> 514:     // The Runtime1::handle_exception_from_callee_id handler is invoked after the
> 515:     // frame has been unwinded. It instead builds its own stub frame, to call the

Unwided -> unwound, maybe? Same below and probably other places as well.

src/hotspot/share/compiler/oopMap.cpp line 243:

> 241:   } else {
> 242:     all_do(fr, reg_map, f, process_derived_oop, &do_nothing_cl);
> 243:   }

I wonder if we shouldn't hide the StackWatermarkSet in the GC code, and not "activate" the DerivedPointerTable when a
SWS is used? Isn't it the case that already don't enable the table for ZGC? Couldn't this simply be: `
  if (DerivedPointerTable::is_active()) {
    all_do(fr, reg_map, f, add_derived_oop, &do_nothing_cl);
  } else {
    all_do(fr, reg_map, f, process_derived_oop, &do_nothing_cl);
  }
`

src/hotspot/share/compiler/oopMap.cpp line 312:

> 310: #ifdef ASSERT
> 311:         if ((((uintptr_t)loc & (sizeof(oop)-1)) != 0) ||
> 312:             !Universe::heap()->is_in_or_null((oop)NativeAccess<AS_NO_KEEPALIVE>::oop_load(&val))) {

Discussed offline. This is technical debt that we'd like to get rid of somehow.

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

PR: https://git.openjdk.java.net/jdk/pull/296