RFR: 8154153: PS: Restore preserved marks in parallel

Thomas Schatzl thomas.schatzl at oracle.com
Fri Apr 22 13:01:08 UTC 2016


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



More information about the hotspot-gc-dev mailing list