Hi Kim, >> 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.
Ok, done: https://bugs.openjdk.java.net/browse/JDK-8214441 >>>> - 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? https://bugs.openjdk.java.net/browse/JDK-8204947?focusedCommentId=14226307&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14226307 Thanks for reviewing, Roman
