[11u] RFR: 8244214: Add paddings for TaskQueuSuper to reduce false-sharing cache contention
goetz.lindenmaier at sap.com
Fri Jun 26 07:17:24 UTC 2020
I had a look at your change.
I think it makes sense to bring this to 11, if there actually is
the performance gain you mention.
Please add in the "Fix request" comment in the JBS the risk
of downporting this. And I think is should be "Fix request (11u)"
because different people will review your fix request for 11 and 8.
> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces at openjdk.java.net> On
> Behalf Of Patrick Zhang OS
> Sent: Wednesday, June 24, 2020 11:55 AM
> To: jdk-updates-dev at openjdk.java.net
> Cc: hotspot-gc-dev <hotspot-gc-dev at openjdk.java.net>
> Subject: [DMARC FAILURE] [11u] RFR: 8244214: Add paddings for
> TaskQueuSuper to reduce false-sharing cache contention
> Could I ask for a review of this simple patch which takes a tiny part from the
> original ticket JDK-8243326 . The reason that I do not want a full backport
> is, the majority of the patch at jdk/jdk  is to clean up the volatile use and
> may be not very meaningful to 11u, furthermore the context (dependencies
> on atomic.hpp refactor) is too complicated to generate a clear backport (I
> tried, ~81 files need to be changed).
> The purpose of having this one-line change to 11u is, the two volatile
> variables in TaskQueuSuper: _bottom, _age and corresponding atomic
> operations upon, may cause severe cache contention inside GC with larger
> number of threads, i.e., specified by -XX:ParallelGCThreads=##, adding
> paddings (up to DEFAULT_CACHE_LINE_SIZE) in-between can reduce the
> possibility of false-sharing cache contention. I do not need the paddings
> before _bottom and after _age from the original patch , because the
> instances of TaskQueuSuper are usually (always) allocated in a set of queues,
> in which they are naturally separated. Please review, thanks.
> JBS: https://bugs.openjdk.java.net/browse/JDK-8248214
> Webrev: http://cr.openjdk.java.net/~qpzhang/8248214/webrev.01/
> Testing: tier1-2 pass with the patch, commercial benchmarks and small C++
> test cases (to simulate the data struct and work-stealing algorithm atomics)
> validated the performance, no regression.
> By the way, I am going to request for 8u backport as well once 11u would
> have it.
>  https://bugs.openjdk.java.net/browse/JDK-8243326 Cleanup use of
> volatile in taskqueue code
>  https://hg.openjdk.java.net/jdk/jdk/rev/252a1602b4c6
More information about the hotspot-gc-dev