[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8183542: Factor out serial GC specific code from GenCollectedHeap into its own subclass
From:       Roman Kennke <rkennke () redhat ! com>
Date:       2017-10-25 8:07:53
Message-ID: 90d877fb-a406-e1e3-6d26-a5e00e5666eb () redhat ! com
[Download RAW message or body]

Hi Kim, hi Jini,

thank you both for your reviews!

I think I need a sponsor now. The final webrev (same as before plus 
Reviewed-by line):
http://cr.openjdk.java.net/~rkennke/8183542/webrev.03/ 
<http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.03/>

Thanks, Roman

> > On Oct 19, 2017, at 12:39 PM, Roman Kennke <rkennke@redhat.com> wrote:
> > 
> > Am 18.10.2017 um 22:41 schrieb Kim Barrett:
> > > > On Oct 18, 2017, at 4:04 PM, Roman Kennke <rkennke@redhat.com> wrote:
> > > > 
> > > > Am 18.10.2017 um 20:41 schrieb Kim Barrett:
> > > > > > On Oct 18, 2017, at 8:08 AM, Roman Kennke <rkennke@redhat.com> wrote:
> > > > > > Differential webrev:
> > > > > > http://cr.openjdk.java.net/~rkennke/8183542/webrev.01.diff/ \
> > > > > > <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01.diff/> 
> > > > > > Full webrev:
> > > > > > http://cr.openjdk.java.net/~rkennke/8183542/webrev.01/ \
> > > > > > <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.01/> 
> > > > > > Better now?
> > > > > > 
> > > > > > Thanks, Roman
> > > > > Looks good.
> > > > > 
> > > > Hi Kim,
> > > > 
> > > > thanks for the review.
> > > > 
> > > > I just fixed a bug caused by my similar CMSHeap extraction, and I think I \
> > > > need to do the same thing for SerialHeap too: 
> > > > https://bugs.openjdk.java.net/browse/JDK-8189373
> > > > 
> > > > This is the fix for the CMSHeap issue:
> > > > 
> > > > http://cr.openjdk.java.net/~rkennke/8189373/webrev.00/ \
> > > > <http://cr.openjdk.java.net/%7Erkennke/8189373/webrev.00/> 
> > > > I'll do the same for SerialHeap once the above has been approved and pushed, \
> > > > otherwise it'll be a mess. ;-) 
> > > > Roman
> > > The SA strikes again!  Yes, it looks like the same thing should be done for \
> > > SerialHeap. I'm going to leave the review of 8189373 to others who have more \
> > > clue about the SA. 
> > Okidoki, so here comes the SerialGC with SA boilerplate:
> > 
> > Differential:
> > http://cr.openjdk.java.net/~rkennke/8183542/webrev.02.diff/ \
> > <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.02.diff/> 
> > Full:
> > http://cr.openjdk.java.net/~rkennke/8183542/webrev.02/ \
> > <http://cr.openjdk.java.net/%7Erkennke/8183542/webrev.02/> 
> > This builds on top of the patch for \
> > https://bugs.openjdk.java.net/browse/JDK-8189373 which should land in the repo \
> > shortly, and implements the same thing for SerialHeap. It also passes the test \
> > that failed in the mentioned bug report (with -XX:+UseSerialGC). 
> > Can I get reviews (for the changed/added stuff) again?
> > 
> > Thanks, Roman
> Looks good to me, though I don't know much about the SA.
> It at least looks consistent with the changes for JDK-8189373.
> 


[prev in list] [next in list] [prev in thread] [next in thread]