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