RFR: 8240872: Shenandoah: Avoid updating new regions from start of evacuation

Roman Kennke rkennke at redhat.com
Wed Mar 11 17:47:09 UTC 2020


Right. I was thinking of constructor and asserts right after I've posted
the RFR. Fixed this and all else that you've mentioned:

http://cr.openjdk.java.net/~rkennke/JDK-8240872/webrev.02/

Good now?

Roman

On 3/11/20 6:11 PM, Aleksey Shipilev wrote:
> On 3/11/20 6:08 PM, Roman Kennke wrote:
>> http://cr.openjdk.java.net/~rkennke/JDK-8240872/webrev.01/
> 
> Good. Mostly stylistic nits.
> 
> *) Rename this local in shenandoahHeap.cpp?
> 
> 2420       HeapWord* top_at_start_ur = r->get_update_watermark();
> 
> *) shenandoahHeapRegion.cpp: excess semicolon at L435, and probably argument should be just "w"?
> That would make the code more obvious without highlighting. You'd probably want to assert in
> set_update_watermark that it is in bounds [bottom, top]?
> 
>  433   HeapWord* get_update_watermark() const {
>  434     return _update_watermark;
>  435   };
>  436
>  437   void set_update_watermark(HeapWord* update_watermark) {
>  438     _update_watermark = update_watermark;
>  439   }
> 
> *) shenandoahHeapRegion.cpp. Call set_update_watermark in recycle() to capture the asserts.
> 
>  482   _update_watermark = bottom();
> 
> *) New field should be initialized in ShenandoahHeapRegion::ShenandoahHeapRegion? I don't think we
> call recycle() before using the region.
> 



More information about the shenandoah-dev mailing list