RFR (S) 8220350: Refactor ShenandoahHeap::initialize

Roman Kennke rkennke at redhat.com
Fri Mar 8 18:57:12 UTC 2019


Hi Aleksey,

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
+      );

Also, your webrev seems inconsistent:
e.g.: 
http://cr.openjdk.java.net/~shade/8220350/webrev.01/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp.udiff.html

vs.: 
http://cr.openjdk.java.net/~shade/8220350/webrev.01/src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp.patch

and:
http://cr.openjdk.java.net/~shade/8220350/webrev.01/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp.udiff.html

vs

http://cr.openjdk.java.net/~shade/8220350/webrev.01/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp.patch

I am not sure I am looking at the correct stuff! :-)

Roman


> RFE:
>    https://bugs.openjdk.java.net/browse/JDK-8220350
> 
> Fix:
>    http://cr.openjdk.java.net/~shade/8220350/webrev.01/
> 
> This prepares Shenandoah code for accepting bugfixes in this method. Some asserts are added to
> verify important fields are indeed initialized. Pacer cache moved out of ShHeapRegion to resolve
> initialization circularity -- it does not matter anyway, because increase_live_data_gc_words is
> called rarely itself, being protected by the cache during mark.
> 
> Testing: hotspot_gc_shenandoah
> 
> Thanks,
> -Aleksey
> 



More information about the hotspot-gc-dev mailing list