RFR: Embed traversal_set and root_regions into ShenandoahTraversalGC to avoid derefs

Roman Kennke rkennke at redhat.com
Thu Jun 28 10:43:26 UTC 2018


>>>> 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.

Ok, let's do it that way then:
http://cr.openjdk.java.net/~rkennke/traversal-embed-stuff/webrev.01/

OK?

Roman



More information about the shenandoah-dev mailing list