RFR: 8261838: Shenandoah: reconsider heap region iterators memory ordering

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8261838: Shenandoah: reconsider heap region iterators memory ordering

Aleksey Shipilev-5
We use CASes to distributed workers between regions. Hotspot's default for atomic operations is memory_order_conservative, which emits two-way memory fences around the CASes at least on AArch64 and PPC64.

This seems to be excessive for region distribution code, and "relaxed" is enough, since we don't piggyback memory ordering on these.

This also calls for some refactoring in the code itself.

Additional testing:
 - [x] `hotspot_gc_shenandoah`
 - [ ] Ad-hoc performance runs

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

Commit messages:
 - 8261838: Shenandoah: reconsider heap region iterators memory ordering

Changes: https://git.openjdk.java.net/jdk/pull/2593/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2593&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261838
  Stats: 24 lines in 4 files changed: 2 ins; 3 del; 19 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2593.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2593/head:pull/2593

PR: https://git.openjdk.java.net/jdk/pull/2593
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261838: Shenandoah: reconsider heap region iterators memory ordering

Roman Kennke
On Tue, 16 Feb 2021 19:13:03 GMT, Aleksey Shipilev <[hidden email]> wrote:

> We use CASes to distributed workers between regions. Hotspot's default for atomic operations is memory_order_conservative, which emits two-way memory fences around the CASes at least on AArch64 and PPC64.
>
> This seems to be excessive for region distribution code, and "relaxed" is enough, since we don't piggyback memory ordering on these.
>
> This also calls for some refactoring in the code itself.
>
> Additional testing:
>  - [x] `hotspot_gc_shenandoah`
>  - [ ] Ad-hoc performance runs

Looks good!

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

Marked as reviewed by rkennke (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2593
Reply | Threaded
Open this post in threaded view
|

Integrated: 8261838: Shenandoah: reconsider heap region iterators memory ordering

Aleksey Shipilev-5
In reply to this post by Aleksey Shipilev-5
On Tue, 16 Feb 2021 19:13:03 GMT, Aleksey Shipilev <[hidden email]> wrote:

> We use CASes to distributed workers between regions. Hotspot's default for atomic operations is memory_order_conservative, which emits two-way memory fences around the CASes at least on AArch64 and PPC64.
>
> This seems to be excessive for region distribution code, and "relaxed" is enough, since we don't piggyback memory ordering on these.
>
> This also calls for some refactoring in the code itself.
>
> Additional testing:
>  - [x] `hotspot_gc_shenandoah`
>  - [ ] Ad-hoc performance runs

This pull request has now been integrated.

Changeset: fd098e71
Author:    Aleksey Shipilev <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/fd098e71
Stats:     24 lines in 4 files changed: 2 ins; 3 del; 19 mod

8261838: Shenandoah: reconsider heap region iterators memory ordering

Reviewed-by: rkennke

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

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