RFR(M): Backports of G1 bugs 8009940, 8008301, 8010463, 8010780, 8012335, and 8012715 to hs24

[Bengt Rutisson]

Hi John,

Thanks for providing both the old and new webrevs. Makes this much 
easier to review!

Comments inline...

On 5/2/13 9:04 PM, John Cuthbertson wrote:
> Hi Everyone,
>
> Some more backports that did not apply cleanly to review:
>
> *8009940:****G1: assert(_finger == _heap_end) failed, 
> concurrentMark.cpp:809*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.2.8009940.hsx24.patch/
> Original: http://cr.openjdk.java.net/~johnc/8009940/webrev.1/
>
> The changes did not apply cleanly because of CMTask::_worker_id being 
> renamed to CMTask::_task_id.

Looks good.

>
> Note these changes are generated w.r.t the backports for 8005032 and 
> 8009536 being applied.
>
> *8010463: G1: Crashes with -UseTLAB and heap verification*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.4.8010463.hsx24.patch/
> Original: http://cr.openjdk.java.net/~johnc/8010463/webrev.0/
>
> I'm not sure why this did not apply cleanly but the rejected chunk was 
> in g1CollectedHeap.cpp.

In g1CollectedHeap.cpp it did not apply cleanly because of changes for 
perm gen removal.

I think the original (hs25) webrev may not be up-to-date.

The tests in the webrevs are different:

http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.4.8010463.hsx24.patch/test/gc/TestVerifyBeforeGCDuringStartup.java.html
http://cr.openjdk.java.net/~johnc/8010463/webrev.0/test/gc/TestVerifyBeforeGCDuringStartup.java.html

But your backported test looks like the one that was actually pushed to 
hs25:

http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/file/24ef5fb05e0f/test/gc/TestVerifyBeforeGCDuringStartup.java 


Similarly, what you actually pushed to hs25 in thread.cpp looks more 
like what you propose to backport to hs24:

http://hg.openjdk.java.net/hsx/hotspot-gc/hotspot/diff/24ef5fb05e0f/src/share/vm/runtime/thread.cpp
http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.4.8010463.hsx24.patch/src/share/vm/runtime/thread.cpp.udiff.html

So, I think this looks good.

>
> *8010780: G1: Eden occupancy/capacity output wrong after a full GC*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.5.8010780.hsx24.patch/
> Original: http://cr.openjdk.java.net/~johnc/8010780/webrev.0/
>
> The changes (especially in g1CollectedHeap.cpp) conflicted with the 
> tracing changes.

A bit difficult to review since the patch files are quite different. As 
far as I can tell it looks good.

>
> The following webrevs applied cleanly to hsx24, so no review is needed 
> but I'm including them for completeness.

Did not look at these. Since they apply cleanly I'm sure their fine.

All in all: Looks good! Ship it!

Bengt


>
> *8008301: **G1: guarantee(satb_mq_set.completed_buffers_num() == 0) 
> failure*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.3.8008301.hsx24.patch/
> Original: http://cr.openjdk.java.net/~johnc/8008301/webrev.0/
>
> Note this webrev is was applied on top of 8005032, 8009536, and 8009940.
>
> *8012335: G1: Fix bug with compressed oops in template interpreter on 
> x86 and sparc.*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.6.8012335.hsx24.patch/
> Original: http://cr.openjdk.java.net/~goetz/webrevs/g1-cOops_bug/
>
> *8012715: G1: GraphKit accesses PtrQueue::_index as int but is size_t*
> Backport: 
> http://cr.openjdk.java.net/~johnc/hsx24-backports/webrev.7.8012715.hsx24.patch/
> Original: http://cr.openjdk.java.net/~goetz/webrevs/8012715/
>
> Note: the last two were contributed by Martin Doerr from SAP.
>
> Thanks,
>
> JohnC
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130502/b63978db/attachment.htm>