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

[Claes Redestad]

On Thu, 22 Oct 2020 19:48:19 GMT, Сергей Цыпанов <github.com+10835776+stsypanov at openjdk.org> wrote:

>>> On 15/10/2020 11:41, Claes Redestad wrote:
>>> 
>>> > In the short term we could consider a point fix to replace existing
>>> > use of `new AtomicInteger/-Long(0)` with `new
>>> > AtomicInteger/-Long()`, but in the long term we should really try
>>> > and optimize our JITs so that writing to a volatile field in
>>> > constructors can be made cheaper.
>>> 
>>> I guess it all depends on whether there's a happens-before
>>> relationship between the volatile store to Field X in a constructor
>>> and any subsequent get() of Field X in another thread. Intuitively I
>>> would have thought so, but I can't find any guarantee in the spec.
>>> Nonetheless, I think we'd have little gain by changing this and it
>>> might break (arguably incorrect) programs.
>> 
>> I misphrased somewhat: I think we should try and remove explicit stores of default values to volatile fields in constructors, as suggested by https://bugs.openjdk.java.net/browse/JDK-8145948 
>> 
>> I can't think of a way this would break any program (even arguably incorrect ones), and would net us the gains shown in comments here without any point fixes.
>
> @cl4es could I ask one more question? Would it be helpful to create a separate PR about removal of explicit zeroing of Atomic* classes (excluding AtomicBoolean), e.g. somethin like this https://github.com/openjdk/jdk/compare/master...stsypanov:atom-simpl ?

> @cl4es could I ask one more question? Would it be helpful to create a separate PR about removal of explicit zeroing of Atomic* classes (excluding AtomicBoolean), e.g. somethin like this [master...stsypanov:atom-simpl](https://github.com/openjdk/jdk/compare/master...stsypanov:atom-simpl) ?

Sure. Fixing `static final`s is unlikely to matter much, but it's a trivial change and as long as it has a performance advantage it's good to be consistent.

General advice is to limit sweeping changes to related modules, and not include code maintained in some upstream repo. What you have here looks fine.

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

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