RFR: Embed traversal_set and root_regions into ShenandoahTraversalGC to avoid derefs

[Roman Kennke]

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.

Roman