RFR: 8254146: Avoid unnecessary volatile write on new AtomicBoolean(false)

[Claes Redestad]

On Wed, 7 Oct 2020 13:38:36 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> Hi,
>> 
>> the following PR optimizes `new AtomicBoolean(boolean)` by avoiding the volatile write in case `false` is passed.
>> Essentially, it changes the ternary operator to a simple `if` without the `else` that would cause the volatile write.
>> The resulting bytecode seems to also benefit from the change:
>>     Code:
>>        0: aload_0
>>        1: invokespecial #1                  // Method java/lang/Object."<init>":()V
>>        4: aload_0
>>        5: iload_1
>>        6: ifeq          13
>>        9: iconst_1
>>       10: goto          14
>>       13: iconst_0
>>       14: putfield      #7                  // Field value:I
>>       17: return
>> 
>> After:
>>     Code:
>>        0: aload_0
>>        1: invokespecial #1                  // Method java/lang/Object."<init>":()V
>>        4: iload_1
>>        5: ifeq          13
>>        8: aload_0
>>        9: iconst_1
>>       10: putfield      #7                  // Field value:I
>>       13: return
>> 
>> A simple benchmark that returns `new AtomicBoolean(false)` shows the following results, that brings it on par to `new
>> AtomicBoolean()`: MyBenchmark.empty                                         avgt   10     3,103 ±   0,246   ns/op
>> MyBenchmark.explicitNew                                   avgt   10     2,966 ±   0,071   ns/op
>> MyBenchmark.explicitOld                                   avgt   10     7,738 ±   0,321   ns/op
>> 
>> In case you think this is worthwhile I'd be happy if this is sponsored.
>> Cheers,
>> Christoph
>
> Marked as reviewed by rriggs (Reviewer).

> @AlanBateman What would be the label (or workflow) for that now?

@dreis2211 what Alan reminded us of here is that all of j.u.concurrent is maintained upstream from the OpenJDK, and
that this change needs to be coordinated with the maintainers there. Either by getting an OK to go ahead and push this
as-is, or by contributing this patch to the JSR166 repo directly (which will then be integrated back into the OpenJDK
at some point). See http://gee.cs.oswego.edu/dl/concurrency-interest/

This isn't anything new since the GitHub migration: Various bits and pieces of the OpenJDK sources are maintained
upstream, and it's not always easy to igure out exactly where or how to go about upstreaming a change.

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

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