April 2016 - core-libs-dev

RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them
by Steve Drach 29 Apr '16

29 Apr '16

Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 <https://bugs.openjdk.java.net/browse/JDK-8151542> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ <http://cr.openjdk.java.net/~sdrach/8151542/webrev/> This changeset causes the URL returned from a ClassLoader.getResource(name) invocation to be reified, that is the URL is a direct pointer to either a versioned or unversioned entry in the jar file. The patch also assures that jar URL’s are always processed by the URLClassPath.JarLoader. The MultiReleaseJarURLConnection test was enhanced to demonstrate that reified URLs are returned. The SimpleHttpServer test helper class was moved into it’s own file.

5 13

Integer/Long reverse bits optimization
by Jaroslav Kameník 29 Apr '16

29 Apr '16

Hello! I have a small patch to Integer and Long classes, which is speeding up bit reversion significantly. Last two/three steps of bit reversion are doing byte reversion, so there is possibility to use intrinsified method reverseBytes. Java implementation of reverseBytes is similar to those steps, so it should give similar performance when intrinsics are not available. Here I have result of few performance tests (thank for hints, Aleksej:) : old code: # VM options: -server ReverseInt.reverse 1 avgt 5 8,766 ± 0,214 ns/op ReverseLong.reverse 1 avgt 5 9,992 ± 0,165 ns/op # VM options: -client ReverseInt.reverse 1 avgt 5 9,168 ± 0,268 ns/op ReverseLong.reverse 1 avgt 5 9,988 ± 0,123 ns/op patched: # VM options: -server ReverseInt.reverse 1 avgt 5 6,411 ± 0,046 ns/op ReverseLong.reverse 1 avgt 5 6,299 ± 0,158 ns/op # VM options: -client ReverseInt.reverse 1 avgt 5 6,430 ± 0,022 ns/op ReverseLong.reverse 1 avgt 5 6,301 ± 0,097 ns/op patched, intrinsics disabled: # VM options: -server -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l ReverseInt.reverse 1 avgt 5 9,597 ± 0,206 ns/op ReverseLong.reverse 1 avgt 5 9,966 ± 0,151 ns/op # VM options: -client -XX:DisableIntrinsic=_reverseBytes_i,_reverseBytes_l ReverseInt.reverse 1 avgt 5 9,609 ± 0,069 ns/op ReverseLong.reverse 1 avgt 5 9,968 ± 0,075 ns/op You can see, there is little slowdown in integer case with intrinsics disabled. It seems to be caused by different 'shape' of byte reverting code in Integer.reverse and Integer.reverseByte. I tried to replace reverseByte code with piece of reverse method, and it is as fast as not patched case: ReverseInt.reverse 1 avgt 5 9,184 ± 0,255 ns/op Diffs from jdk9: Integer.java: @@ -1779,9 +1805,8 @@ i = (i & 0x55555555) << 1 | (i >>> 1) & 0x55555555; i = (i & 0x33333333) << 2 | (i >>> 2) & 0x33333333; i = (i & 0x0f0f0f0f) << 4 | (i >>> 4) & 0x0f0f0f0f; - i = (i << 24) | ((i & 0xff00) << 8) | - ((i >>> 8) & 0xff00) | (i >>> 24); - return i; + + return reverseBytes(i); Long.java: @@ -1940,10 +1997,8 @@ i = (i & 0x5555555555555555L) << 1 | (i >>> 1) & 0x5555555555555555L; i = (i & 0x3333333333333333L) << 2 | (i >>> 2) & 0x3333333333333333L; i = (i & 0x0f0f0f0f0f0f0f0fL) << 4 | (i >>> 4) & 0x0f0f0f0f0f0f0f0fL; - i = (i & 0x00ff00ff00ff00ffL) << 8 | (i >>> 8) & 0x00ff00ff00ff00ffL; - i = (i << 48) | ((i & 0xffff0000L) << 16) | - ((i >>> 16) & 0xffff0000L) | (i >>> 48); - return i; + + return reverseBytes(i); Jaroslav

2 2

Re: RFR(m): 8140281 deprecate Optional.get()
by Stuart Marks 29 Apr '16

29 Apr '16

On 4/27/16 1:39 AM, Dr Heinz M. Kabutz wrote: > thanks for these excellent examples of where Optional.get() is being used > incorrectly in the JDK. I would like to publish a Java Specialists' > Newsletter about this topic and hopefully educate at least a part of the Java > proletariat on how to do it correctly. To my embarassment, this is the first > time that I've /seen/ Optional.ifPresent(), so I include myself amongst the > proles. I think it was just too close to Optional.isPresent() and I just > overlooked it. However, the longer, clumsier name would not IMHO make any > difference. I would rather ask the IDE vendors to build in autofix mechanisms > to transform the code in these examples to what it should look like. You're welcome. I'm also a bit embarrassed at finding this code in the JDK. I guess we need to do some more internal education. On the name, I'd be happy to find a less clumsy name that conveys the programmer's assertion that a value is always present. I also think it's reasonable, independently of any deprecation/renaming, for IDE vendors to add rule checkers that attempt to detect unsafe uses of Optional. I'm a bit skeptical of whether it's possible for there to be automatic fixes, but detection would be a good step forward. > Now a code style question. Let's say that I get back an Optional<BigInteger> > and I currently do the following with it: > > Optional<BigInteger> prime = ... > if (prime.isPresent()) { > System.out.println("Prime is " + prime.get()); > } else { > System.out.println("Prime not found"); > } Assuming the Optional<BigInteger> came from a method called findPrime(), I'd rewrite this code as follows: System.out.println(findPrime().map(p -> "Prime is " + p) .orElse("Prime not found")); > Is there a better way of doing this? We'd almost need a > ifPresentElse(Consumer<? super T> consumer, Runnable action) method I guess. Close. In JDK 9 we added ifPresentOrElse(Consumer<? super T> action, Runnable emptyAction) This is useful if you really need to perform separate actions in the different cases, instead of just substituting or transforming values. > Now, another question, do I have your permission to reference the examples of > incorrect usage below and quote you on how it should be done in my newsletter? Sure, I'd love to see your writeup on this. This is all open source and open email logs, so there aren't any secrets here. There are a couple caveats though: 1) There are places in the JDK code where special conditions apply. For example, early in startup, we avoid using lambdas because it drags in all the lambda support code prematurely. In such cases my suggested changes wouldn't be appropriate. I think this kind of issue doesn't apply to most applications and libraries, though. 2) I don't claim to have the final word on the "right" way to use Optional. It is, unfortunately, all too easy to find examples of poor usage of Optional.get(). I've suggested some fixes that I like, but others might prefer different approaches that are equally valid. s'marks

5 6

RFR: 8155600 - Performance optimization of Arrays.asList().iterator()
by Tagir F. Valeev 29 Apr '16

29 Apr '16

Hello! Please review and sponsor the following patch: https://bugs.openjdk.java.net/browse/JDK-8155600 http://cr.openjdk.java.net/~tvaleev/webrev/8155600/r1/ It appears that custom implementation of Arrays.asList().iterator() is noticeably more performant than the AbstractList implementation used currently. Here's JMH test which illustrates this: http://cr.openjdk.java.net/~tvaleev/webrev/8155600/jmh/ @Benchmark public int sumList() { return sum(list); } @Benchmark public int sumArray() { return sum(Arrays.asList(arr)); } public static int sum(Iterable<Integer> data) { int sum = 0; for (int item : data) sum+=item; return sum; } Here's summary result on my Intel x64 machine: JDK 9ea+115: Benchmark (limit) Mode Cnt Score Error Units ArrayTest.sumArray 0 avgt 50 6,371 ± 0,009 ns/op ArrayTest.sumArray 1 avgt 50 8,020 ± 0,100 ns/op ArrayTest.sumArray 10 avgt 50 14,424 ± 0,060 ns/op ArrayTest.sumArray 100 avgt 50 88,684 ± 3,261 ns/op ArrayTest.sumArray 1000 avgt 50 677,048 ± 1,050 ns/op ArrayTest.sumArray 10000 avgt 50 8038,224 ± 59,678 ns/op ArrayTest.sumList 0 avgt 50 3,395 ± 0,003 ns/op ArrayTest.sumList 1 avgt 50 5,613 ± 0,061 ns/op ArrayTest.sumList 10 avgt 50 13,448 ± 0,076 ns/op ArrayTest.sumList 100 avgt 50 97,242 ± 0,105 ns/op ArrayTest.sumList 1000 avgt 50 664,342 ± 0,704 ns/op ArrayTest.sumList 10000 avgt 50 7971,230 ± 61,344 ns/op JDK 9ea+115 patched: Benchmark (limit) Mode Cnt Score Error Units ArrayTest.sumArray 0 avgt 50 2,932 ± 0,011 ns/op ArrayTest.sumArray 1 avgt 50 5,000 ± 0,062 ns/op ArrayTest.sumArray 10 avgt 50 9,421 ± 0,219 ns/op ArrayTest.sumArray 100 avgt 50 43,848 ± 0,187 ns/op ArrayTest.sumArray 1000 avgt 50 383,619 ± 1,019 ns/op ArrayTest.sumArray 10000 avgt 50 6860,087 ± 13,022 ns/op ArrayTest.sumList 0 avgt 50 3,400 ± 0,008 ns/op ArrayTest.sumList 1 avgt 50 6,142 ± 0,047 ns/op ArrayTest.sumList 10 avgt 50 12,488 ± 0,084 ns/op ArrayTest.sumList 100 avgt 50 83,029 ± 0,078 ns/op ArrayTest.sumList 1000 avgt 50 605,651 ± 0,992 ns/op ArrayTest.sumList 10000 avgt 50 7649,681 ± 16,355 ns/op AbstractList.iterator() makes unnecessary bookkeeping like modCount tracking which reduces the performance. Also it seems that having implicit this$0 field in AbstractList.iterator() makes impossible to avoid allocation of short-lived Arrays.ArrayList object in sumArray() test. Launching JMH -prof gc confirms that non-patched sumArray() test allocates 24 bytes per invocation while patched sumArray() does not allocate anything. What do you think? Is it reasonable change? Should I check some tests or add new ones for this optimization? With best regards, Tagir Valeev.

6 10

RFR 8154755: Add a VarHandle weakCompareAndSet with volatile semantics
by Paul Sandoz 29 Apr '16

29 Apr '16

Hi, Please review: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8154755-weak-cas-volatile/webr… This webrev adds a new VarHandle access mode method, weakCompareAndSetVolatile [*]. (I also fixed some doc errors regarding AccessMode enum names that i forgot to update in a previous issue). Aleksey noticed a discrepancy with the current set of weak CAS methods when comparing with C++ atomics. On hardware that supports LL/SC (e.g. ARM) the weak volatile variant may be more performant than the strong volatile variant (compareAndSet) when the CAS is performed in a loop. See the thread on the jmm-dev list for more details: http://mail.openjdk.java.net/pipermail/jmm-dev/2016-April/000239.html Implementations currently defer to the strong volatile Unsafe CAS. We will follow up with changes to Unsafe, hotspot, and updating the VH implementations as separate issues. Thanks, Paul. [*] /** * Possibly atomically sets the value of a variable to the {@code newValue} * with the memory semantics of {@link #setVolatile} if the variable's * current value, referred to as the <em>witness value</em>, {@code ==} the * {@code expectedValue}, as accessed with the memory semantics of * {@link #getVolatile}. * * <p>This operation may fail spuriously (typically, due to memory * contention) even if the witness value does match the expected value. * ... */ public final native @MethodHandle.PolymorphicSignature @HotSpotIntrinsicCandidate boolean weakCompareAndSetVolatile(Object... args); Paul.

3 2

RFR (XS) 8155090: String concatenation fails with a custom SecurityManager that uses concatenation
by Aleksey Shipilev 29 Apr '16

29 Apr '16

Hi, Please review the fix for a shady bootstrapping issue, when a custom SecurityManager is using string concatenation: https://bugs.openjdk.java.net/browse/JDK-8155090 The essence of the issue is that during StringConcatFactory::<clinit>, we are reading the system properties via the privileged calls. When user SecurityManager that uses string concatenation is set, we are trying to produce a string concatenation stub in order to proceed, and double-back on SCF. There, we try to run SCF methods without fully complete <clinit>: the existing test fails with uninitialized static final Strategy field. The cleanest (yet subtle) solution here is to make sure the default SCF settings are good to run with, which allows transient <clinit> operations to complete normally: http://cr.openjdk.java.net/~shade/8155090/webrev.00/ Testing: offending test; java/lang/String jtregs Thanks, -Aleksey

3 3

JDK 9 pre-review of JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract
by joe darcy 28 Apr '16

28 Apr '16

Hello, With talk of deprecation in the air [1], I thought it would be a fine time to examine one of the bugs on my list JDK-6850612: Deprecate Class.newInstance since it violates the checked exception language contract As the title of the bug implies, The Class.newInstance method knowingly violates the checking exception contract. This has long been documented in its specification: > Note that this method propagates any exception thrown by the nullary > constructor, including a checked exception. Use of this method > effectively bypasses the compile-time exception checking that would > otherwise be performed by the compiler. The Constructor.newInstance > method avoids this problem by wrapping any exception thrown by the > constructor in a (checked) InvocationTargetException. Roughly, the fix would be to turn the text of this note into the @deprecated text and to add a @Deprecated(since="9") annotation to the method. There are a few dozen uses of the method in the JDK that would have to be @SuppressWarning-ed or otherwise updated. Thoughts on the appropriateness of deprecating this method at this time? Comments on the bug have suggested that besides deprecating the method, a new method on Class could be introduced, newInstanceWithProperExceptionBehavior, that had the same signature but wrapped exceptions thrown by the constructor call in the same way Constructor.newInstance does. Thanks, -Joe [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040192.html

10 14

RFR (XS) 8155215: java.lang.String concatenation spec is unnecessarily strong
by Aleksey Shipilev 28 Apr '16

28 Apr '16

Hi, Please review this tiny spec improvement that syncs up JLS and java.lang.String Javadoc. java.lang.String Javadoc unnecessarily locks down the implementation details for String concat (e.g. usage of StringBuilder): http://cr.openjdk.java.net/~shade/8155215/webrev.00/ I'll submit CCC as soon as we agree on wording. Thanks, -Aleksey

3 3

RFR [9] 8155578: OpenJDK build failed after JDK-8044773
by Chris Hegarty 28 Apr '16

28 Apr '16

This is a review for a trivial, but build-fatal, addition of a qualified export to the jdk.net module, that was mistakenly omitted from the changes for 8044773. $ hg diff -U 11 diff --git a/src/java.base/share/classes/module-info.java b/src/java.base/share/classes/module-info.java --- a/src/java.base/share/classes/module-info.java +++ b/src/java.base/share/classes/module-info.java @@ -158,22 +158,23 @@ exports jdk.internal.misc to java.corba, java.desktop, java.logging, java.management, java.naming, java.rmi, java.security.jgss, java.sql, java.xml, jdk.charsets, + jdk.net, jdk.scripting.nashorn, jdk.unsupported, jdk.vm.ci; exports jdk.internal.perf to java.desktop, java.management, jdk.jvmstat; exports jdk.internal.ref to java.desktop; exports jdk.internal.reflect to java.corba, -Chris.

2 1

RFR (JAXP) 8154220 : Semi-colon delimited list of catalog files in System property is throwing IllegalArgumentException
by huizhe wang 27 Apr '16

27 Apr '16

Hi, Please review the following issue and fix: JBS: https://bugs.openjdk.java.net/browse/JDK-8154220 webrev: http://cr.openjdk.java.net/~joehw/jdk9/8154220/webrev/ Thanks, Joe

2 4