RFR: 8146989: Introduce per-worker preserved mark stacks in ParNew

[Thomas Schatzl]

Hi,

  sorry for the delay.

On Wed, 2016-01-13 at 19:07 -0500, Tony Printezis wrote:
> This is the change that introduces per-worker preserved mark stacks
> for ParNew (please see the previous thread on this):
> 
> http://cr.openjdk.java.net/~tonyp/8146989/webrev.1/

- I would prefer if the code were structured differently. I do not like
the additional remove_forwarding_pointers() in ParNewGeneration, and
the call to DefNew::remove_forwarding_pointers() in the way it's done
right now.

Considering how mark restoration will likely be implemented in the
other collectors (like in
ParNewGeneration::remove_forwarding_pointers()), it looks like there
will be a lot of code duplication involved.

So my suggestion is to add a PreservedMarksSet class that is as the
name implies basically a set of PreservedMarks.

Apart from getting references to the individual PreservedMarks (to pass
to ParScanThreadState), it has one virtual method that restores the
marks. In the default implementation, it just iterates over all
PreservedMarks elements of the set, calling PreservedMark::restore().

Add one particular instance to DefNewGeneration - in case of ParNew
usage initialize it with #parallel threads PreservedMarks, in case of
DefNew just one.

Pass PreservedMarks[i] to the ParScanThreadState.

I think similar things can be done for Parallel GC (and G1).

Individual GCs can then override PreservedMarksSet::restore_all_marks()
that does what is currently written as a note in
ParScanThreadSet::restore_preserved_marks(), depending on collector.

Btw, I do not think it is useful to write such long notes into the
code. Further enhancement ideas should be stored in the bug tracker.
Very very few people are browsing through that code, particular CMS, to
find work to do :)

That PreservedMarksSet class, or whatever you want to call it, would
roughly correspond to G1CollectedHeap::_preserved_objs.

> A couple of notes:
> 
> * Please see the note on the JIRA re: why the preserved mark stack on
> DefNew might still be used when ParNew is used.

I think above proposal is better, adding a single virtual method to
some kind of PreservedMarksSet class that can be overridden by the
collector.

This keeps this particular evacuation failure functionality private to
that PreservedMarksSet implementation and not somewhere in the already
huge CollectedHeap subclasses (for such an "unimportant" feature).

> * I moved the OopAndMarkOop and OopAndMarkOopStack classes from the
> G1 file to the shared file so I can re-use them. I thought doing this
> it’s cleaner than including the G1 file in the shared file.
> 
> * I’ll re-use the PreservedMarks class for the ParallelGC changes too
> (see JDK-8146991). In fact, it might be worth also using it in G1
> too, so that all the logic is in one place. Your call.

Yes, we should reuse the code everywhere as much as possible (and
reasonable).

Other minor comments:

- the copyright dates are all wrong :), and for the new files, they
should only contain the year 2016.

Thanks,
  Thomas