RFR(s): 8198267: Allow GCId::current_raw() calls from non-NamedThreads
kim.barrett at oracle.com
Tue Feb 20 06:32:00 UTC 2018
> On Feb 19, 2018, at 8:08 AM, Per Liden <per.liden at oracle.com> wrote:
> Got some off-line feedback from StefanK. After some discussions we decided to also make the following changes:
> 1) Renamed current_raw() to current_or_undefined()
> 2) Replaces a few uses of current_raw() with currentNamedThread->gc_id()
> 3) Removes the special constructor for AbstractGangTask, which is no longer needed. With the new semantics of current_or_undefined() the default constructor can be used by G1PretouchTask.
> Webrev: http://cr.openjdk.java.net/~pliden/8198267/webrev.1/
> On 02/16/2018 02:30 PM, Per Liden wrote:
>> Patch to allow GCId::current_raw() calls from non-NamedThreads. This is needed when, for example, an Access barrier executed by a Java thread calls into GC-code.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8198267
>> Webrev: http://cr.openjdk.java.net/~pliden/8198267/webrev.0/
The inability to call GCId::current_raw from non-NamedThreads is what
led me to jump through some hoops in the gtests for OopStorage,
sometimes needing to transition a thread from native to VM.
That led me to look at the OopStorage gtest again, and now I'm
wondering how this proposed change even compiles? It replaces the
two-argument constructor for AbstractGangTask with a one-argument
constructor, but doesn't update the calls in test_oopStorage.cpp that
pass two arguments.
I think I would prefer that AbstractGangTask defaulted to
GCId::current() and let specific callers that need to do so instead
specify GCId::undefined(), or maybe current_or_undefined().
I also note that this change dropped the
Universe::is_fully_initialized() test in G1PretouchTask. Is that
effectively subsumed by the Thread::current() in
current_or_undefined(). Or should that be calling
Thread::current_or_null and checking the result first? Or perhaps that
check is obsolete?
BTW, the paranoid / hygenic would make the now one-argument
constructor "explicit", to prevent unintended implicit conversions.
Yeah, I don't always remember that either.
More information about the hotspot-gc-dev