RFR: Backout: Refactor evac-in-progress flag to more general gc-phase flag

Aleksey Shipilev shade at redhat.com
Mon Oct 2 16:03:55 UTC 2017


On 10/02/2017 04:50 PM, Roman Kennke wrote:
> Am 02.10.2017 um 16:17 schrieb Aleksey Shipilev:
>> On 10/02/2017 04:06 PM, Roman Kennke wrote:
>>> This is a clean backout of the previous change "Refactor evac-in-progress flag to more general
>>> gc-phase flag". In discussions we came to the conclusion that we should leave that code as it was,
>>> and instead make new experimental code check for existing flags.
>>>
>>> Ok to back out that change?
>>>
>>> http://cr.openjdk.java.net/~rkennke/backout-gcflags/webrev.00/
>> *) This looks wrong, should be just cbz? If that was masked by the patch, it would be nice to fix it
>> up in another change after this backout.
>>
>> (two places in current patch):
>> -  tbz(rscratch1, ShenandoahHeap::EVACUATION_BITPOS, done);
>> +  cbzw(rscratch1, done);
>>
>> Otherwise looks good. Thanks for doing this!
> 
> It's how it was before. It is not wrong, it's just not quite as beautiful. The preceding load loads
> a byte, and masks off the upper bits. cbzw compares the lower 32bits and ignores the upper 32bits.
> cbz would consider all 64bits. If I understand correctly. Shouldn't make a difference
> performance-wise or correctness-wise, but I agree that cbz would look better.
> 
> Ok to push? And fix this in a followup change?

Yes to both.

-Aleksey




More information about the shenandoah-dev mailing list