RFR: 8260941: Remove the conc_scan parameter for CardTable

[Thomas Schatzl-4]

Hi,

  can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?

The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.

In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).

This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).

Testing: tier1-5

-------------

Commit messages:
 - Remove scan_concurrently()
 - Compilation fixes
 - First attempt to prune code

Changes: https://git.openjdk.java.net/jdk/pull/2425/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2425&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8260941
  Stats: 81 lines in 17 files changed: 4 ins; 66 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2425.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2425/head:pull/2425

PR: https://git.openjdk.java.net/jdk/pull/2425

[Albert Mingkun Yang]

On Fri, 5 Feb 2021 09:52:25 GMT, Thomas Schatzl <[hidden email]> wrote:

> Hi,
>
>   can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?
>
> The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.
>
> In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).
>
> This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).
>
> Testing: tier1-5

A side note: it seems that `G1BarrierSet` is the only subclass of `CardTableBarrierSet`. Maybe it makes sense to merge them into one.

-------------

Marked as reviewed by ayang (Author).

PR: https://git.openjdk.java.net/jdk/pull/2425

[Kim Barrett]

> On Feb 10, 2021, at 4:44 AM, Albert Mingkun Yang <[hidden email]> wrote:
>
> On Fri, 5 Feb 2021 09:52:25 GMT, Thomas Schatzl <[hidden email]> wrote:
>
>> Hi,
>>
>>  can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?
>>
>> The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.
>>
>> In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).
>>
>> This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).
>>
>> Testing: tier1-5
>
> A side note: it seems that `G1BarrierSet` is the only subclass of `CardTableBarrierSet`. Maybe it makes sense to merge them into one.

GenCollectedHeap (i.e. SerialGC) directly instantiates CardTableBarrierSet.

[Kim Barrett-2]

On Fri, 5 Feb 2021 09:52:25 GMT, Thomas Schatzl <[hidden email]> wrote:

> Hi,
>
>   can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?
>
> The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.
>
> In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).
>
> This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).
>
> Testing: tier1-5

Looks good.  Just the one minor nit about "virtual".

src/hotspot/share/gc/g1/g1BarrierSet.hpp line 56:

> 54:   ~G1BarrierSet() { }
> 55:
> 56:   bool card_mark_must_follow_store() const {

Should be "virtual".  (Probably should be "override", but nobody's gotten around to vetting and proposing that change to the style guide.)

-------------

Marked as reviewed by kbarrett (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2425

[Thomas Schatzl-4]

> Hi,
>
>   can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?
>
> The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.
>
> In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).
>
> This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).
>
> Testing: tier1-5

Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:

  kbarrett review

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2425/files
  - new: https://git.openjdk.java.net/jdk/pull/2425/files/5db8cbe6..9c0129d3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2425&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2425&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2425.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2425/head:pull/2425

PR: https://git.openjdk.java.net/jdk/pull/2425

[Thomas Schatzl-4]

On Wed, 10 Feb 2021 09:42:15 GMT, Albert Mingkun Yang <[hidden email]> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   kbarrett review
>
> A side note: it seems that `G1BarrierSet` is the only subclass of `CardTableBarrierSet`. Maybe it makes sense to merge them into one.

Thanks for your revies @albertnetymk @kimbarrett

-------------

PR: https://git.openjdk.java.net/jdk/pull/2425

[Thomas Schatzl-4]

On Fri, 5 Feb 2021 09:52:25 GMT, Thomas Schatzl <[hidden email]> wrote:

> Hi,
>
>   can I have reviews for this removal of the last(?) CMS-specific code in CardTable, namely some provision to indicate that cards are being scanned concurrently in Serial/Parallel GC barrier code?
>
> The change simply follows the predicate into Serial/Parallel GC code which always returns false for them and removes that code.
>
> In the review for JDK-8234534 I mentioned that I split this out due to unexplainable errors; testing tier1-5 three times showed none of that any more (after updating to latest code).
>
> This change has only been built on Oracle-platforms and linux-x86 via github actions (https://github.com/tschatzl/jdk/actions/runs/539993964), so I would like to kindly ask maintainers of the others to compile and report issues (32 bit ARM, PPC etc).
>
> Testing: tier1-5

This pull request has now been integrated.

Changeset: 9c0ec8d8
Author:    Thomas Schatzl <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/9c0ec8d8
Stats:     81 lines in 17 files changed: 4 ins; 66 del; 11 mod

8260941: Remove the conc_scan parameter for CardTable

Reviewed-by: ayang, kbarrett

-------------

PR: https://git.openjdk.java.net/jdk/pull/2425