RFR(L): 8195142: Refactor out card table from CardTableModRefBS to flatten the BarrierSet hierarchy

[Erik Österlund]

Hi,

Here is an updated revision of this webrev after internal feedback from 
StefanK who helped looking through my changes - thanks a lot for the 
help with that.

The changes to the new revision are a bunch of minor clean up changes, 
e.g. copy right headers, indentation issues, sorting includes, 
adding/removing newlines, reverting an assert error message, fixing 
constructor initialization orders, and things like that.

The problem I mentioned last time about the version number of our repo 
not yet being bumped to 11 and resulting awkwardness in JVMCI has been 
resolved by simply waiting. So now I changed the JVMCI logic to get the 
card values from the new location in the corresponding card tables when 
observing JDK version 11 or above.

New full webrev (rebased onto a month fresher jdk-hs):
http://cr.openjdk.java.net/~eosterlund/8195142/webrev.01/

Incremental webrev (over the rebase):
http://cr.openjdk.java.net/~eosterlund/8195142/webrev.00_01/

This new version has run through hs-tier1-5 and jdk-tier1-3 without any 
issues.

Thanks,
/Erik

On 2018-01-17 13:54, Erik Österlund wrote:
> Hi,
>
> Today, both Parallel, CMS and Serial share the same code for its card 
> marking barrier. However, they have different requirements how to 
> manage its card tables by the GC. And as the card table itself is 
> embedded as a part of the CardTableModRefBS barrier set, this has led 
> to an unnecessary inheritance hierarchy for CardTableModRefBS, where 
> for example CardTableModRefBSForCTRS and CardTableExtension are 
> CardTableModRefBS subclasses that do not change anything to do with 
> the barriers.
>
> To clean up the code, there should really be a separate CardTable 
> hierarchy that contains the differences how to manage the card table 
> from the GC point of view, and simply let CardTableModRefBS have a 
> CardTable. This would allow removing CardTableModRefBSForCTRS and 
> CardTableExtension and their references from shared code (that really 
> have nothing to do with the barriers, despite being barrier sets), and 
> significantly simplify the barrier set code.
>
> This patch mechanically performs this refactoring. A new CardTable 
> class has been created with a PSCardTable subclass for Parallel, a 
> CardTableRS for CMS and Serial, and a G1CardTable for G1. All 
> references to card tables and their values have been updated accordingly.
>
> This touches a lot of platform specific code, so would be fantastic if 
> port maintainers could have a look that I have not broken anything.
>
> There is a slight problem that should be pointed out. There is an 
> unfortunate interaction between Graal and hotspot. Graal needs to know 
> the values of g1 young cards and dirty cards. This is queried in 
> different ways in different versions of the JDK in the 
> ||GraalHotSpotVMConfig.java file. Now these values will move from 
> their barrier set class to their card table class. That means we have 
> at least three cases how to find the correct values. There is one for 
> JDK8, one for JDK9, and now a new one for JDK11. Except, we have not 
> yet bumped the version number to 11 in the repo, and therefore it has 
> to be from JDK10 - 11 for now and updated after incrementing the 
> version number. But that means that it will be temporarily 
> incompatible with JDK10. That is okay for our own copy of Graal, but 
> can not be used by upstream Graal as they are given the choice whether 
> to support the public JDK10 or the JDK11 that does not quite admit to 
> being 11 yet. I chose the solution that works in our repository. I 
> will notify Graal folks of this issue. In the long run, it would be 
> nice if we could have a more solid interface here.
>
> However, as an added benefit, this changeset brings about a hundred 
> copyright headers up to date, so others do not have to update them for 
> a while.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8195142
>
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8195142/webrev.00/
>
> Testing: mach5 hs-tier1-5 plus local AoT testing.
>
> Thanks,
> /Erik