8152312: ParNew: Restore preserved marks in parallel (original) (raw)
Thomas Schatzl thomas.schatzl at oracle.com
Tue Mar 22 10:49:15 UTC 2016
- Previous message (by thread): 8152312: ParNew: Restore preserved marks in parallel
- Next message (by thread): 8152312: ParNew: Restore preserved marks in parallel
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Hi tony,
On Mon, 2016-03-21 at 11:58 -0400, Tony Printezis wrote:
Hi all,
I was advised to get the ParNew changes reviewed separately from the G1 changes (see 8151556). Here are just the PreservedMarks* changes + the ParNew change: http://cr.openjdk.java.net/~tonyp/8152312/webrev.0/
Thanks for this change.
I’ll redo the G1 changes for 8151556 to depend on this change.
I also piggy-backed the following change (for DefNew): logdebug(gc)("Promotion failed”); -> loginfo(gc, promotion)("Promotion failed”); so that it’s consistent with what ParNew does. Any objections?
No :)
Some comments however:
- I would prefer if the decision to do parallel/serial mark restoration were hidden in PreservedMarksSet.
It simplifies the use of the class (no need to even think about whether the correct method is called).
Just pass the WorkGang to the restore() method. There still can be separate, private parallel/serial mark restoration methods.
Further, arbitrary decisions on when to use what method can be made (e.g. because I am pretty sure in many cases using all worker threads is not a good idea, e.g. for almost or completely empty lists) without the need to change anything in the callers.
Additionally the assert_empty and the log message do not need to be duplicated at the end of restore and par_restore.
please use size_t for ParRestoreTask::_total_size. There is an Atomic::add() for size_t.
preservedMarks.cpp:95: the comment is a proper sentence, please start with upper case and end with full stop.
and finally the bike-shedding topic (feel free to ignore), I would prefer to not use the "Par" prefix to the ParRestoreTask task. It's kind of redundant because these tasks should be kind of agnostic in how many workers are used.
:)
Thanks, Thomas
- Previous message (by thread): 8152312: ParNew: Restore preserved marks in parallel
- Next message (by thread): 8152312: ParNew: Restore preserved marks in parallel
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]