RFR (XXS): 7006810: G1: Introduce peace-of-mind checking in the Suspendible Thread Set

[Srinivas Ramakrishna]

Hi Joe --

I am not terribly familiar with G1 code yet, but here's a suggestion:

* rename the boolean field to "in_suspendible_thread_set" (or
"suspendible_thread"), the corresponding setters and gettes to
set_in_suspendible_thread_set(), clear_in_suspendible_thread_set(), and
is_in_suspendible_thread_set(), or preferably:
set_suspendible_thread(), clear_suspendible_thread() and
is_suspendible_thread(). (I don't particularly like the set_...._set()
naming.)

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
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
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
join/leave abstraction via SuspendibleThreadSetJoiner, but there also seem
to be a bunch of naked join/leave calls, so the
extra checking would likely be worthwhile.

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
as well, since the frequency of transition to and from STS mode is probably
fairly low, and the extra information may be useful for
field serviceability. But I will let those more familiar with G1 opine more
strongly on that.

Looks good otherwise.

Thanks!
-- ramki (openjdk: ysr)



On Mon, Apr 6, 2015 at 10:25 AM, Joseph Provino <joseph.provino at oracle.com>
wrote:

> CR:  https://bugs.openjdk.java.net/browse/JDK-7006810
>
> webrev:  http://cr.openjdk.java.net/~jprovino/7006810/webrev.00
>
> test:  jprt
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150406/48e6a638/attachment.htm>