Request for code review (M) - 8081629: CMS split_block() does not correctly fix up block-offset-table for large blocks

Thomas Schatzl thomas.schatzl at oracle.com
Wed Jul 8 10:34:54 UTC 2015


Hi Jon,

  sorry for the late review...

On Mon, 2015-06-22 at 12:57 -0700, Jon Masamitsu wrote:
> This bug is causing large young GC times for ParNew when
> there are large objects in the heap and the logarithmic strides
> are being used (badly).
> 
> https://bugs.openjdk.java.net/browse/JDK-8081629
> 
> The change in webrev_min corrects the problem
> with the minimum code change (least risk).
> 
> http://cr.openjdk.java.net/~jmasa/8081629/webrev_min.02/
> 
> A rewriting of split_block() in included in a second webrev
> 
> http://cr.openjdk.java.net/~jmasa/8081629/webrev.02/
> 
> I've willing to go with the minimum change but could also
> be encouraged to push the rewrite of split_block().
> 
> Vote for the one you like with your code review.

Voting for webrev.02 as it seems easier to understand to me.

I would suggest to fix one comment in both version that read 

         // Fill in the remainder of this "power block", if it
         // is non-null.

replacing non-null by "empty". A "null" range seems confusing to me in
this context.

Both seem okay to me (with some extra spaces in line 469 that should be
fixed before pushing in the "min" fix).

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list