RFR 8146991: Introduce per-worker preserved mark stacks in ParallelGC

Thomas Schatzl thomas.schatzl at oracle.com
Thu Mar 3 14:35:12 UTC 2016


Hi Tony,

On Tue, 2016-03-01 at 13:22 -0500, Tony Printezis wrote:
> Hi all,
> 
> These are the per-worker preserved mark stack changes for ParallelGC:
> 
> http://cr.openjdk.java.net/~tonyp/8146991/webrev.0/
> 
> They are similar to the ParNew changes and rely on some classes I
> introduced then.
> 

- In PSPromotionManager::initialize(), please extract the
"ParallelGCThreads + 1" calculation into a local variable.

- nitpick: the comment in PSPromotionManager::post_scavenge(), since it is a proper sentence, start with upper case and end with full stop.

- in PreservedMarksSet::restore(), I would prefer if the total_size
were calculated as the marks are restored. I do not see a particular
reason why you would need to calculate this beforehand, and it is an
extra loop over a potentially large number of threads.

Like PreservedMarks::get() returned the number of preserved marks, and
the method just added them together.

- I personally dislike the use of "counter-variable += 1" in the
increment section of for-loops, and prefer using a post-decrement. Feel
free to ignore this comment.

> Quick question on this: I moved:
> 
> log_trace(gc, ergo)("Restoring " SIZE_FORMAT " marks", total_size());
> 
> to the shared code so that the message is logged irrespective of GC
> (for example, I don’t think we were logging this for ParNew or
> DefNew). Is “ergo” the right category here though?

- I would remove the "ergo" tag, and try to make the message more
meaningful, as the "gc" tag is the catch-all bucket. E.g. something
like "Restoring... marks after evacuation failure.".

Otherwise it looks good to me. I will run it through jprt and some test
list.

Thanks,
  Thomas



More information about the hotspot-gc-dev mailing list