RFR: Embed traversal_set and root_regions into ShenandoahTraversalGC to avoid derefs

[Aleksey Shipilev]

On 06/28/2018 10:47 AM, Roman Kennke wrote:
> Am 28.06.2018 um 10:26 schrieb Aleksey Shipilev:
>> On 06/27/2018 05:34 PM, Roman Kennke wrote:
>>> I've noticed some unnecessary derefs in hot marking/traversal loops to
>>> get to stuff like cset, mark-bitmap and traversal set.
>>>
>>> Here I'm proposing to embed the traversal_set and root_regions into
>>> ShenandoahTraversalGC. This helps to avoid some of those derefs. I
>>> changed all access to it to use the embedded object directly inside
>>> ShTraversalGC, and only hand it out as reference. Only exception is the
>>> ShHeapRegionSetIterator, which still converts it to pointer internally,
>>> because we can't reset a reference.
>>>
>>> The patch also makes ShHeap and task-queues const in ShTraversalGC.
>>>
>>> http://cr.openjdk.java.net/~rkennke/traversal-embed-stuff/webrev.00/
>>
>> *) Why the changes to ShenandoahHeapRegionSetIterator needed? Can we keep ShHeapRegionSet* in its
>> interface, and only convert to pointer when needed? This would keep the SHRS* usages intact.
>> *) It also seems simpler to return the addresses right away here, and then just deal with SHRS*
>> everywhere, as usual. E.g. instead of:
>>
>>   ShenandoahHeapRegionSet& traversal_set() { return _travers_set; }
>>   ShenandoahHeapRegionSet& root_regions()  { return _root_regions;}
>>
>> Do:
>>
>>   ShenandoahHeapRegionSet* traversal_set() { return &_traversal_set; }
>>   ShenandoahHeapRegionSet* root_regions()  { return &_root_regions;  }
>>
>> This should trim down the patch significantly, and optimizing compilers should see through it:
>>   https://godbolt.org/g/HZ2hAv
> 
> 
> Counter-question: why not? C++ references are generally nicer, when
> possible (e.g. guaranteed not-null), consistent in usage to 'flat'
> objects, and otherwise pretty much the same as ptrs in most interesting
> regards. I'd like to use more of this in our code and trim down raw
> pointer usage instead. No strong preference on my side though, just
> curiosity.

Since references are implemented as pointers anyway, there seem to be no particular benefit in
trying to use references everywhere. Keeping the raw pointers makes the style more consistent with
the rest of Shenandoah and Hotspot: grep for "return &", it is a usual style to keep the class
member "inlined", but access it via the pointer. I would prefer not to deviate from that style,
unless there is a very compelling reason to do so -- and there seem to be no such reason.

-Aleksey