<div dir="ltr">Hi Joe --<div><br></div><div>I am not terribly familiar with G1 code yet, but here's a suggestion:<div><br></div><div>* rename the boolean field to "in_suspendible_thread_set" (or "suspendible_thread"), the corresponding setters and gettes to</div><div>set_in_suspendible_thread_set(), clear_in_suspendible_thread_set(), and is_in_suspendible_thread_set(), or preferably:</div><div>set_suspendible_thread(), clear_suspendible_thread() and is_suspendible_thread(). (I don't particularly like the set_...._set() naming.)</div><div><br></div><div>Do we allow threads to recursively enter the suspendible thread set in nested fashion? I'm guessing we don't, in which case, you would</div><div>use this information local to the thread to prevent/forbid such recursive entries or exits when it isn't part of the set, by asserting</div><div>that the bit isn't set when a thread joins the set and never clear before a thread leaves the set. I understand that there's a scoped</div><div>join/leave abstraction via SuspendibleThreadSetJoiner, but there also seem to be a bunch of naked join/leave calls, so the</div><div>extra checking would likely be worthwhile.</div><div><br></div><div>As Eric notes, you could make all the new fields/methods NOT_PRODUCT, but I am not too concerned about leaving them in product mode</div><div>as well, since the frequency of transition to and from STS mode is probably fairly low, and the extra information may be useful for</div><div>field serviceability. But I will let those more familiar with G1 opine more strongly on that.</div><div><br></div><div>Looks good otherwise.</div><div><br></div><div>Thanks!</div><div>-- ramki (openjdk: ysr)</div><div><br></div><div><br></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 6, 2015 at 10:25 AM, Joseph Provino <span dir="ltr"><<a href="mailto:joseph.provino@oracle.com" target="_blank">joseph.provino@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">CR:  <a href="https://bugs.openjdk.java.net/browse/JDK-7006810" target="_blank">https://bugs.openjdk.java.net/<u></u>browse/JDK-7006810</a><br>
<br>
webrev:  <a href="http://cr.openjdk.java.net/~jprovino/7006810/webrev.00" target="_blank">http://cr.openjdk.java.net/~<u></u>jprovino/7006810/webrev.00</a><br>
<br>
test:  jprt<br>
</blockquote></div><br></div>