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

[Srinivas Ramakrishna]

Hi Jon --

Sorry for the looong delay in getting to this and then of turning around
the webrev once I started looking at the code.
It's been a while since I was in this code, and the code is quite
complicated, so it took a while to swap back
the context and understand (the correctness of) the change.



On Mon, Jun 22, 2015 at 12:57 PM, Jon Masamitsu <jon.masamitsu at oracle.com>
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/



The above changes look good to me. Thanks also for confirming that these
changes fix the assertion failure seen with +VerifyBlockOffsetArray. It
might be
worthwhile writing (in the future) a slightly more lightweight
VerifyBlockOffsetArray
so that it becomes practical to use.

I haven't had the opportunity to look at and understand the second version
of the
webrev below yet. But the one above looks good to me. I'll try and get to
the
version below in the next day or so. Sorry for the ongoing delay.

thanks!
-- ramki (openjdk: ysr)


>
> 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.
>
> The diff between webrev_min.02 and webrev.02 is
> here but I don't find it that useful.
>
> http://cr.openjdk.java.net/~jmasa/8081629/webrev_delta.02/
>
> Thanks.
>
> Jon
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150831/ab051f7d/attachment.htm>