RFR: 8154153: PS: Restore preserved marks in parallel

[Tony Printezis]

Thomas,

I’ll fix the initializer list, give me a few mins and I’ll post a new webrev.

Tony

On April 22, 2016 at 9:01:16 AM, Thomas Schatzl (thomas.schatzl at oracle.com) wrote:

Hi Tony,  

On Thu, 2016-04-21 at 08:35 -0400, Tony Printezis wrote:  
> Thomas,  
>  
> New webrev:  
>  
> http://cr.openjdk.java.net/~tonyp/8154153/webrev.1/  
>  
> On April 21, 2016 at 8:08:28 AM, Tony Printezis (tprintezis at twitter.c  
> om) wrote:  
> > Thomas,  
> >  
> > Thanks for looking at it. Inline.  
> >  
> > On April 21, 2016 at 7:01:39 AM, Thomas Schatzl (thomas.schatzl at ora  
> > cle.com) wrote:  
> > > Hi Tony,   
> > >  
> > > On Wed, 2016-04-13 at 09:24 -0400, Tony Printezis wrote:   
> > > > Follow-up change to do preserved mark restoration in parallel  
> > > in PS:   
> > > >   
> > > > http://cr.openjdk.java.net/~tonyp/8154153/webrev.0/   
> > > >   
> > > > I also changed the “Promotion failed” log message from:   
> > > > log_info(gc)("Promotion failed”);   
> > > > to:   
> > > > log_info(gc, promotion)("Promotion failed");   
> > > > to be consistent with the other GCs.   
> > > >   
> > >  
> > > - renaming of ParRestoreTask to ParRestoreGangTask: no other   
> > > AbstractGangTask child class has "Gang" in its name.   
> > >  
> > > I can kind of see the name clashes with "ParRestoreGCTask", but I  
> > > do   
> > > not think it really helps.   
> > Yeah, I renamed it to differentiate it a bit from ParRestoreGCTask.  
> > No strong opinion here. If you’re happy with ParRestoreTask /  
> > ParRestoreGCTask we can go with that.  

Either way is fine with me too. Let's keep your suggestion.  

> > >  
> > > - preservedMarks.cpp: line 118, maybe an extra CR makes the code  
> > > look   
> > > less cramped.   
> > Between the name() and do_it() methods?  
> >  

Yes.  

> > >  
> > > - preservedMarks.cpp: line 128: style: if argument list needs to  
> > > be   
> > > split across multiple lines, we (in the gc team) favor one  
> > > argument per   
> > > line.   
> > Sure, sounds good and I’ll keep that in mind in the future. Both at  
> > the method declaration and definition?  

Both.  

> > Does that also apply to the initializer list?  

Yes.  

> > > - some copyrights need updates   
> > Will do. I’ll post a new version shortly.  

The change above looks good. I can fix the initializer list if you  
don't just update the webrev in place :)  

Thanks,  
  Thomas  
-----

Tony Printezis | JVM/GC Engineer / VM Team | Twitter

@TonyPrintezis
tprintezis at twitter.com

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