> On Nov 27, 2018, at 4:46 AM, Roman Kennke <rken...@redhat.com> wrote: > > Hi Kim, > >>> Sections to review (at this point) are the following: >>> >>> *) shared-gc >>> - This is mostly boilerplate that is common to any GC >>> - referenceProcessor.cpp has a little change to make one assert not fail >>> (next to CMS and G1) >> >> Ick. But I don’t have a better suggestion that doesn’t involve adding a new >> API >> to CollectedHeap for use by this assertion, which seems a bit excessive if >> there >> are no other uses. > > Yeah. > I guess we could add a config _discovery_is_concurrent or similar in RP, > and check that. Or maybe one of _discovery_is_mt or _discovery_is_atomic > already covers that? I couldn't immediately tell/100% understand their > semantics. Seems worthy to look at after this?
It might be equivalent to _discovery_is_atomic; I don’t remember the exact semantics right now. I think it’s unrelated to _discovery_is_mt. Yes, looking at this later is fine. Please file an RFE. >>> - taskqueue.hpp has some small adjustments to enable subclassing >> >> Why this change instead of JDK-8204947? As the description from that RFE >> says: >> "The ShenandoahTaskTerminator from the Shenandoah project is a much better >> implementation of a task terminator.” > > Yeah, see Zhengyu's comment. Let's ignore those changes for this review > (for now), expect our impl ported to taskqueue.hpp/cpp soon. I’m okay with that plan. Maybe add a comment in JDK-8204947 about this?
