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

Claes Redestad redestad at openjdk.java.net
Thu Oct 15 19:08:19 UTC 2020


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

>>> What do you think?
>> 
>> For `AtomicBoolean`, this optimization piggy-backed on an existing test, just rewriting it so that no action is taken
>> when none is needed. So it's never a pessimization, and even improves bytecode size marginally. So it'll be a marginal
>> gain even if the current overhead of writing to a volatile field in a constructor is removed/reduced.   For
>> `AtomicInteger/-Long`, we'd have to add a test that's not there, so we'd pessimize non-zero cases by adding some
>> bytecode and a branch to remove the overhead. An overhead that can already be avoided by calling the empty constructor
>> instead. So I think it's questionable to do this.  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.
>
> @cl4es , @dreis2211 thanks for explanation!

> 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.

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

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


More information about the core-libs-dev mailing list