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


Reply via email to