RFR (S) 8220350: Refactor ShenandoahHeap::initialize
Roman Kennke
rkennke at redhat.com
Fri Mar 8 19:34:24 UTC 2019
>> Instead of re-formatting the constructor call here, I'd probably rather extract the arguments into
>> local variables:
>>
>> - ShenandoahHeapRegion* r = new ShenandoahHeapRegion(this,
>> - (HeapWord*) pgc_rs.base() + reg_size_words
>> * i,
>> - reg_size_words,
>> + ShenandoahHeapRegion* r = new ShenandoahHeapRegion(
>> + this,
>> + (HeapWord*) sh_rs.base() + ShenandoahHeapRegion::region_size_words() * i,
>> + ShenandoahHeapRegion::region_size_words(),
>> i,
>> - i < num_committed_regions);
>> + i < num_committed_regions
>> + );
>
> Would this be better?
>
> for (size_t i = 0; i < _num_regions; i++) {
> HeapWord* start = (HeapWord*)sh_rs.base() + size_words * i;
> bool is_committed = i < num_committed_regions;
> ShenandoahHeapRegion* r = new ShenandoahHeapRegion(this, start, size_words, i, is_committed);
Yes, much better.
>> Also, your webrev seems inconsistent:
>
> Not sure what happened there. Trying again:
> http://cr.openjdk.java.net/~shade/8220350/webrev.02/
Looks good! Thanks!
Roman
More information about the hotspot-gc-dev
mailing list