On Tue, 22 Sep 2020 11:43:41 GMT, Erik Österlund <[email protected]> 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
