RFR (S): 8133047: Rename G1ParScanThreadState::_queue_num to _worker_id

Jon Masamitsu jon.masamitsu at oracle.com
Wed Aug 5 17:03:06 UTC 2015


http://cr.openjdk.java.net/~tschatzl/8133047/webrev.1/src/share/vm/gc/g1/g1CollectedHeap.cpp.udiff.html

-  assert(_worker_id == _par_scan_state->queue_num(), "sanity");
+  assert(_worker_id == _par_scan_state->worker_id(), "sanity");



Kind of makes you wonder why G1ParClosureSuper needs its own _worker_id
when it has _par_scan_state.  Or is removal of _worker_id from 
G1ParClosureSuper
a change that coming soon?  If not, I'll file a CR.

class G1ParClosureSuper : public OopsInHeapRegionClosure {
protected:
   G1CollectedHeap* _g1;
   G1ParScanThreadState* _par_scan_state;
   uint _worker_id;

Looks good.

Jon




On 8/5/2015 7:35 AM, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2015-08-05 at 16:26 +0200, Jesper Wilhelmsson wrote:
>> Looks good.
>>
>> There is another use of queue_num in
>> G1CollectedHeap::preserve_mark_during_evac_failure(). Was that one intentionally
>> left?
>>
> Overlooked, the code was not in G1ParScanThreadState so I did not look.
> I do think that again worker id is expected here.
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8133047/webrev.1 (full)
> http://cr.openjdk.java.net/~tschatzl/8133047/webrev.0_to_1 (diff)
>
> Thanks,
>    Thomas
>
>
>



More information about the hotspot-gc-dev mailing list