RFR: 8154153: PS: Restore preserved marks in parallel

[Tony Printezis]

Thomas,

Thanks for looking at it. Inline.

On April 21, 2016 at 7:01:39 AM, Thomas Schatzl (thomas.schatzl at oracle.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.




Feel free to ignore this, but maybe thinking about this again yields 
some better names. 

- preservedMarks.cpp needs a memory/resourceArea.hpp include due to the 
use of ResourceMark. 


OK.




- preservedMarks.cpp: line 118, maybe an extra CR makes the code look 
less cramped. 


Between the name() and do_it() methods?




- 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? Does that also apply to the initializer list?




- some copyrights need updates 


Will do. I’ll post a new version shortly.



Tony




I will run it through jprt/some tests. 

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/20160421/25627b39/attachment.htm>