[PATCH] remove redundant initialization of volatile fields with default values

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

Hello Remi,

thanks for pointing this out, I didn't take this into account.

As I understand, the volatile semantics here covers all the fields of an object, 
no matter whether they are declared before or after volatile field (probably
actual object layout might be different from declaration in source code).

If that's true I think we still can apply this optimization at least for

1) classes with single field and without super-classes
2) classes with all non-volatile fields declared final
3) classes with not-initialized non-volatile fields
4) classes with super-classes where non-volatile fields are not initialized (or inialized with default values)

Looking at source code I see that we could keep java.security.KeyStore and 
its nested class PasswordProtection along with java.util.ListResourceBundle.

Please correct me if I miss anything in my speculation.

Regards,
Sergey Tsypanov

19.06.2020, 10:04, "Remi Forax" <forax at univ-mlv.fr>:
> Hi Sergei,
> the problem is that you are changing the semantics if there are several fields.
>
> By example with the code below, you have the guarantee that the code will print 4 (if it prints something),
> if you remove the assignment field = false, the code can print 0 or 4.
>
>   class A {
>     int i = 4;
>     volatile boolean field = false;
>   }
>
> thread 1:
>   global = new A()
>
> thread 2:
>   var a = global;
>   if (a != null) {
>     System.out.println(a.i);
>   }
>
> regards,
> Rémi
>
> ----- Mail original -----
>>  De: "Сергей Цыпанов" <sergei.tsypanov at yandex.ru>
>>  À: "core-libs-dev" <core-libs-dev at openjdk.java.net>
>>  Envoyé: Vendredi 19 Juin 2020 06:57:25
>>  Objet: [PATCH] remove redundant initialization of volatile fields with default values
>
>>  Hello,
>>
>>  while investigating an issue I've found out that assignment of default value to
>>  volatile fields slows down object instantiation.
>>
>>  Consider the benchmark:
>>
>>  @State(Scope.Thread)
>>  @OutputTimeUnit(TimeUnit.NANOSECONDS)
>>  @BenchmarkMode(value = Mode.AverageTime)
>>  @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>>  public class VolatileFieldBenchmark {
>>   @Benchmark
>>   public Object explicitInit() {
>>     return new ExplicitInit();
>>   }
>>
>>   @Benchmark
>>   public Object noInit() {
>>     return new NoInit();
>>   }
>>
>>   private static class ExplicitInit {
>>     private volatile boolean field = false;
>>   }
>>   private static class NoInit {
>>     private volatile boolean field;
>>   }
>>  }
>>
>>  This gives the following results as of my machine:
>>
>>  Benchmark Mode Cnt Score Error Units
>>  VolatileFieldBenchmark.explicitInit avgt 40 11.087 ± 0.140 ns/op
>>  VolatileFieldBenchmark.noInit avgt 40 3.367 ± 0.131 ns/op
>>
>>  I've looked into source code of java.base and found out several cases where the
>>  default value is assigned to volatile field.
>>
>>  Getting rid of such assignements demonstates improvement as of object
>>  instantiation, e.g. javax.security.auth.Subject:
>>
>>            Mode Cnt Score Error Units
>>  before avgt 40 35.933 ± 2.647 ns/op
>>  after avgt 40 30.817 ± 2.384 ns/op
>>
>>  As of testing tier1 and tier2 are both ok after the changes.
>>
>>  Best regards,
>>  Sergey Tsypanov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: volatile.patch
Type: text/x-diff
Size: 1269 bytes
Desc: not available
URL: <https://mail.openjdk.java.net/pipermail/core-libs-dev/attachments/20200619/1ec42cbc/volatile.patch>