RFR: 8145680: Remove unnecessary explicit initialization of volatile variables in java.base
Chris Hegarty
chris.hegarty at oracle.com
Mon Dec 21 10:06:35 UTC 2015
Thanks for doing this Claes. The changes look good to me.
-Chris.
On 20/12/15 23:06, Claes Redestad wrote:
> Hi all,
>
> On 2015-12-20 20:43, Ulf wrote:
>> Hi Claes,
>>
>> I had a very short look and found in j.l.Thread:
>> 211 * Java thread status for tools, 0 indicate thread 'not yet
>> started'
>> I guess you meant:
>> 211 * Java thread status for tools; 0 indicates: thread 'not yet
>> started'.
>>
>> -Ulf
>
> /*
> * Java thread status for tools, default indicates thread 'not yet
> started'
> */
>
> OK?
>
> On 2015-12-20 21:02, Jason Mehrens wrote:
>> Claes,
>>
>> For the cases where boolean was being assigned to 'true' (ASSCI and
>> FileLockImpl) does it hurt performance since the accessor methods will
>> now include a branch at the bytecode level? See: "Speed-kings of
>> inverting booleans" at
>> http://www.javaspecialists.eu/archive/Issue042.html
>>
>> Jason
>
> I don't think this is an issue any more:
>
> public volatile boolean trueValue = true;
> public volatile boolean falseValue = false;
>
> @Benchmark
> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
> public boolean trueWithFalseValue() throws Exception {
> return !falseValue;
> }
>
> @Benchmark
> @CompilerControl(CompilerControl.Mode.DONT_INLINE)
> public boolean trueWithTrueValue() throws Exception {
> return trueValue;
> }
>
> DefaultInitBench.trueWithFalseValue avgt 25 4.538 ± 0.353 ns/op
> DefaultInitBench.trueWithTrueValue avgt 25 4.726 ± 0.466 ns/op
>
> On 2015-12-20 20:33, Joel Borggrén-Franck wrote:
>> Hi Claes,
>>
>> src/java.base/share/classes/java/lang/reflect/Parameter.java
>> src/java.base/share/classes/sun/reflect/MethodAccessorGenerator.java
>> src/java.base/share/classes/sun/reflect/annotation/AnnotationInvocationHandler.java
>>
>>
>> looks fine to me.
>>
>> src/java.base/share/classes/java/lang/Class.java
>>
>> is fine to but isn't
>>
>> enumConstants = constants = temporatyConstants;
>>
>> slightly more idiomatic than
>>
>> - enumConstants = temporaryConstants;
>> + constants = temporaryConstants;
>> + enumConstants = constants;
>
> I'm fine either way; updated in-place in this and a few other places.
>
> Thanks!
>
> /Claes
>
>> On Sun, Dec 20, 2015 at 6:29 PM, Claes Redestad
>> <claes.redestad at oracle.com> wrote:
>>> Hi,
>>>
>>> the changes to java.net.URI stood out as a bit too intrusive for a
>>> cleanup
>>> like this, and there's precious little measurable benefit. I decided to
>>> break out those to a separate RFE and simplified this patch:
>>>
>>> Webrev: http://cr.openjdk.java.net/~redestad/8145680/webrev.02
>>>
>>> /Claes
>>>
>>>
>>> On 2015-12-19 14:07, Claes Redestad wrote:
>>>> Hi,
>>>>
>>>> initializing volatile fields to their default value has a well-known
>>>> performance penalty, and removing these should typically be safe.
>>>> This patch
>>>> addresses java.base.
>>>>
>>>> There are however some corner cases that we need to check for, see
>>>> examples and discussion in
>>>> http://cs.oswego.edu/pipermail/concurrency-interest/2015-December/014767.html
>>>>
>>>>
>>>> When meticulously going through and checking each usage for odd pattern
>>>> like this I accidentally did a bit of extra cleanup, mostly
>>>> addressing a
>>>> number of cases where the volatile field was being read twice. Sorry!
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145680
>>>>
>
More information about the core-libs-dev
mailing list