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

[Сергей Цыпанов]

On Thu, 8 Oct 2020 08:03:12 GMT, Chris Hegarty <chegar 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 chegar (Reviewer).

I appologise for writing this to already closed issue, but Christoph's approach seems to be applicable for at least
`AtomicInteger` and `AtomicLong`, because they are often explicitly zeroed at creation time (see one of my previous PRs
https://github.com/spring-projects/spring-framework/pull/25846) and there we have the same effect as in
`AtomicBoolean`: @State(Scope.Thread) @OutputTimeUnit(TimeUnit.NANOSECONDS) @BenchmarkMode(value = Mode.AverageTime)
public class AtomicBenchmark {
  @Benchmark
  public Object defaultValue() {
    return new AtomicInteger();
  }
  @Benchmark
  public Object explicitValue() {
    return new AtomicInteger(0);
  }
}
Semantically both new AtomicInteger() and new AtomicInteger(0) are the same, but explicitValue() is much slower:
Benchmark                      Mode  Cnt   Score   Error  Units
AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
So the same pattern as we used here could be applied for `AtomicInteger`:
public AtomicInteger(int initialValue) {
  if (initialValue != 0 {
    value = initialValue;
  }
}
What do you think?

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

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