Re: Review Request: JDK-8019227: JDK-8010325 broke the old build
On 27/06/2013 05:37, Brad Wetmore wrote: Brent/Alan/Mike, Hashing.java was removed from the JDK workspace, but was not removed from the old java/java/FILES_java.gmk. Things that still depend on the old build (JCE/deploy) are currently broken. http://cr.openjdk.java.net/~wetmore/8019227/webrev.00/ Brad P.S. I'm very aware that we need to move off the old build soon and am getting closer to finally working on it with Erik, and that the old build isn't complete. But it's complete enough for the JCE dependencies. Unfortunately, this isn't a simple transition (JDK-8006350), and this is a quick fix to get the JCE bits built. The old build has not produced usable bits for several months, it may not have been failing but if you look closely (or run the tests) then you'll see that there are several things missing. On build-dev then you'll probably have seen a mail or two from me where I was trying to encourage the build group to set a date to finally retire and remove the old build - this is because the old build is just a tax on every change that needs to touch the build. Anyway, the change looks okay but I strongly encourage you to put in the time to resolve the usability or others ssues that arise when working on JCE. -Alan
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; } I like this, since it tries to avoid overwriting larger cacheLines with smaller ones when unexistent exponents are requested concurrently for same radix. This is good, since computation for larger exponents takes quadratically longer time (as Alan Eliasen points out) and possibility of concurrent threads stomping on each other increases. Re-reading and cloning powerCache reference at end of computation also takes care of cacheLines for other radixes that might have expanded while the computation was taking place. So the only overhead remaining is when concurrent threads are uselessly computing same results at same time. To avoid this, locking and waiting would have to be introduced which would complicate things. Regards, Peter On 06/26/2013 08:13 PM, Brian Burkhalter wrote: So do we have consensus on this version? Thanks for the lively conversation. Brian On Jun 26, 2013, at 12:05 AM, Aleksey Shipilev wrote: Yes, like that. -Aleksey On 26.06.2013, at 10:53, Dmitry Nadezhin dmitry.nadez...@gmail.com wrote: We could check for the existing cacheLine.length right before installing the new one Do you mean something like this ? BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i exponent + 1; i++) cacheLine[i] = cacheLine[i - 1].square(); if (exponent = powerCache[radix].length) { // volatile read again BigInteger[][] pc = Arrays.copyOf(powerCache); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; }
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
On 06/27/2013 08:37 AM, Peter Levart wrote: Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; } I like this, since it tries to avoid overwriting larger cacheLines with smaller ones when unexistent exponents are requested concurrently for same radix. This is good, since computation for larger exponents takes quadratically longer time (as Alan Eliasen points out) and possibility of concurrent threads stomping on each other increases. Re-reading and cloning powerCache reference at end of computation also takes care of cacheLines for other radixes that might have expanded while the computation was taking place. So the only overhead remaining is when concurrent threads are uselessly computing same results at same time. To avoid this, locking and waiting would have to be introduced which would *complicate things*. Regards, Peter On the other hand, it doesn't complicate thing too much. So if this extra CPU time proves to be a problem, here's a variation of above code which prevents multiple threads from calculating the same result at the same time, by serializing computation with precise granularity: private static class Node { final BigInteger value; Node next; Node(BigInteger value) { this.value = value; } } private static volatile Node[][] powerCache; static { powerCache = new Node[Character.MAX_RADIX + 1][]; for (int i = Character.MIN_RADIX; i = Character.MAX_RADIX; i++) { powerCache[i] = new Node[]{new Node(BigInteger.valueOf(i))}; } } private static BigInteger getRadixConversionCache(int radix, int exponent) { Node[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent].value; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); Node prevNode = cacheLine[oldLength - 1]; for (int i = oldLength; i = exponent; i++) { Node node; synchronized (prevNode) { node = prevNode.next; if (node == null) { node = new Node(prevNode.value.pow(2)); prevNode.next = node; } } cacheLine[i] = prevNode = node; } Node[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent].value; } This variation differs only in a subtelty. It wraps each BigInteger with a Node, which has a link to next node in chain. Computation of next node is serailized using previous node as a lock. So although Nodes can end up in several arrays (which are used as index for quick access), there is only a single growing chain of Nodes per radix and each Node is computed by single thread. Regards, Peter On 06/26/2013 08:13 PM, Brian Burkhalter wrote: So do we have consensus on this version? Thanks for the lively conversation. Brian On Jun 26, 2013, at 12:05 AM, Aleksey Shipilev wrote: Yes, like that. -Aleksey On 26.06.2013, at 10:53, Dmitry Nadezhin dmitry.nadez...@gmail.com wrote: We could check for the existing cacheLine.length right before installing the new one Do you mean something like this ? BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i exponent + 1; i++) cacheLine[i] = cacheLine[i - 1].square(); if (exponent = powerCache[radix].length) { // volatile read again BigInteger[][] pc = Arrays.copyOf(powerCache); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; }
Re: test with a 3rd party jar file?
Hi Chris, Thanks for the information! I'll give it a try. On 6/26/2013 6:18 AM, Chris Hegarty wrote: The streams package recently added tests for exercising package-private implementation. Top level dir contain the TEST.properties to add to the bootclasspath: Do you mean TEST.properties need to be in the top level directory (e.g. javax/xml/jaxp), can it be put anywhere that contains a block of tests? In my particular case, I only want to run a single test with the jar file on the bootclasspath. Thanks, Joe http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/ Example, individual test, whose test is in the java.util.stream package: http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/java/util/stream/DoubleNodeTest.java -Chris. On 25/06/2013 01:51, huizhe wang wrote: Thanks Sean and Rob. Yes, I was told before to avoid shell scripts in tests. I'll wait till after your investigation to see what's the best to do in this case. -Joe On 6/24/2013 3:42 PM, Rob McKenna wrote: Some interesting conversations were had lately about shell scripts during Joe Darcy's recent infrastructure tech talk. In particular around the idea of a jdk.testing package to provide libraries that would help with the types of operations seen in shell scripts. I'm really hoping to spend some time on this myself over the coming weeks. (in an effort to at least understand why we need shell scripts and whether we could do something else instead) In any case it should be possible to simply replace the script with a java program that does the same thing. That would require fiddling with Process however, and its debatable as to whether that would result in fewer test failures. (shell scripts counting for a proportionally large number of those failures) JSR199 might help to reduce the amount of ProcessBuilders required in this instance. -Rob
Question on HashMap change in 8011200
Hi, There are some isEmpty() check added into get/remove methods since 8011200 to return directly if HashMap is empty. However isEmpty is a non-final public method which can be overridden by subclass. If the subclass defines isEmpty differently from HashMap, it would cause problem while getting or removing elements. For example, here is a simple test case, the subclass of HashMap always has at least one key value pair so isEmpty will never be false. /// import java.util.HashMap; public class HashMapTest { public static class NotEmptyHashMapK,V extends HashMapK,V { private K alwaysExistingKey; private V alwaysExistingValue; @Override public V get(Object key) { if (key == alwaysExistingKey) { return alwaysExistingValue; } return super.get(key); } @Override public int size() { return super.size() + 1; } @Override public boolean isEmpty() { return size() == 0; } } public static void main(String[] args) { NotEmptyHashMapString, String map = new NotEmptyHashMap(); map.get(key); } } // The test can end successfully before 8011200 but it will throw ArrayIndexOutOfBoundsException after 8011200. The reason is isEmpty() check in HashMap.getEntry() method gets passed but the actual table length is 0. I think the real intention of isEmpty() check is to check whether size in HashMap is 0 but not whether the subclass is empty, so I suggest to use size == 0 instead of isEmpty() in get/remove methods. Do you think this is a bug or a wrong usage of extending HashMap? I find there is a similar usage in javax.swing.MultiUIDefaults which extends Hashtable. -- Regards, Shi Jun Zhang
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
Hi, Expanding on the idea of building single growing linked list of values and treating the array just as index for quick access which can be re-created at any time without synchronization or volatile publication, here's the attempt: private static class Node { final BigInteger value; Node next; Node(BigInteger value) { this.value = value; } } private static final Node[] powerCache; private static final Node[][] powerCacheIndex; private static final int POWER_CACHE_LINE_CHUNK = 16; static { powerCache = new Node[Character.MAX_RADIX + 1]; for (int i = Character.MIN_RADIX; i = Character.MAX_RADIX; i++) { powerCache[i] = new Node(BigInteger.valueOf(i)); } powerCacheIndex = new Node[Character.MAX_RADIX + 1][]; } private static BigInteger getRadixConversionCache(int radix, int exponent) { Node[] cacheLine = powerCacheIndex[radix]; if (cacheLine != null exponent cacheLine.length) { // cache line is long enough Node node = cacheLine[exponent]; if (node != null) { return node.value; } return fillCacheLine(cacheLine, powerCache[radix], exponent); } else { // we need to extend / create cache line cacheLine = new Node[(exponent / POWER_CACHE_LINE_CHUNK + 1) * POWER_CACHE_LINE_CHUNK]; BigInteger result = fillCacheLine(cacheLine, powerCache[radix], exponent); powerCacheIndex[radix] = cacheLine; // install new line return result; } } private static BigInteger fillCacheLine(Node[] cacheLine, Node node0, int exponent) { Node node = cacheLine[0] = node0; for (int i = 1; i = exponent; i++) { Node nextNode; synchronized (node) { nextNode = node.next; if (nextNode == null) { nextNode = new Node(node.value.square()); node.next = nextNode; } } cacheLine[i] = node = nextNode; } return node.value; } What do you think? Regards, Peter On 06/27/2013 09:27 AM, Peter Levart wrote: On 06/27/2013 08:37 AM, Peter Levart wrote: Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; } I like this, since it tries to avoid overwriting larger cacheLines with smaller ones when unexistent exponents are requested concurrently for same radix. This is good, since computation for larger exponents takes quadratically longer time (as Alan Eliasen points out) and possibility of concurrent threads stomping on each other increases. Re-reading and cloning powerCache reference at end of computation also takes care of cacheLines for other radixes that might have expanded while the computation was taking place. So the only overhead remaining is when concurrent threads are uselessly computing same results at same time. To avoid this, locking and waiting would have to be introduced which would *complicate things*. Regards, Peter On the other hand, it doesn't complicate thing too much. So if this extra CPU time proves to be a problem, here's a variation of above code which prevents multiple threads from calculating the same result at the same time, by serializing computation with precise granularity: private static class Node { final BigInteger value; Node next; Node(BigInteger value) { this.value = value; } } private static volatile Node[][] powerCache; static { powerCache = new Node[Character.MAX_RADIX + 1][]; for (int i = Character.MIN_RADIX; i = Character.MAX_RADIX; i++) { powerCache[i] = new Node[]{new Node(BigInteger.valueOf(i))}; } } private static BigInteger getRadixConversionCache(int radix, int exponent) { Node[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent].value; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); Node prevNode = cacheLine[oldLength - 1]; for (int i =
hg: jdk8/tl/langtools: 7066788: javah again accepts -old option (ineffectively) which was removed in 1.5.
Changeset: a47e28759666 Author:vromero Date: 2013-06-27 09:51 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a47e28759666 7066788: javah again accepts -old option (ineffectively) which was removed in 1.5. Reviewed-by: jjg ! src/share/classes/com/sun/tools/javah/JavahTask.java
hg: jdk8/tl/nashorn: 11 new changesets
Changeset: c30beaf3c42a Author:jlaskey Date: 2013-06-21 14:34 -0300 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/c30beaf3c42a 8010732: BigDecimal, BigInteger and Long handling in nashorn Reviewed-by: sundar Contributed-by: james.las...@oracle.com + test/script/basic/JDK-8010732.js + test/script/basic/JDK-8010732.js.EXPECTED Changeset: 2ded2fc08c94 Author:jlaskey Date: 2013-06-22 10:12 -0300 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/2ded2fc08c94 8017448: JDK-8010732.js.EXPECTED truncated Reviewed-by: sundar Contributed-by: james.las...@oracle.com ! test/script/basic/JDK-8010732.js.EXPECTED Changeset: 51a5ee93d6bc Author:sundar Date: 2013-06-24 19:06 +0530 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/51a5ee93d6bc 8015959: Can't call foreign constructor Reviewed-by: jlaskey, hannesw ! src/jdk/nashorn/api/scripting/JSObject.java ! src/jdk/nashorn/api/scripting/ScriptObjectMirror.java ! src/jdk/nashorn/internal/runtime/ScriptFunction.java ! src/jdk/nashorn/internal/runtime/ScriptFunctionData.java ! src/jdk/nashorn/internal/runtime/ScriptRuntime.java ! src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java + test/script/basic/JDK-8015959.js + test/script/basic/JDK-8015959.js.EXPECTED Changeset: 26a345c26e62 Author:sundar Date: 2013-06-25 17:31 +0530 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/26a345c26e62 8015969: Needs to enforce and document that global context and engine can't be modified when running via jsr223 Reviewed-by: hannesw, jlaskey ! docs/JavaScriptingProgrammersGuide.html ! src/jdk/nashorn/api/scripting/NashornScriptEngine.java ! src/jdk/nashorn/internal/runtime/AccessorProperty.java ! src/jdk/nashorn/internal/runtime/Property.java ! src/jdk/nashorn/internal/runtime/UserAccessorProperty.java + test/script/basic/JDK-8015969.js Changeset: 39e17373d8df Author:sundar Date: 2013-06-26 16:36 +0530 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/39e17373d8df 8017950: error.stack should be a string rather than an array Reviewed-by: hannesw, jlaskey ! src/jdk/nashorn/internal/objects/NativeError.java ! src/jdk/nashorn/internal/runtime/ECMAException.java ! test/script/basic/JDK-8012164.js ! test/script/basic/JDK-8012164.js.EXPECTED + test/script/basic/JDK-8017950.js + test/script/basic/JDK-8017950.js.EXPECTED ! test/script/basic/NASHORN-109.js ! test/script/basic/NASHORN-296.js ! test/script/basic/errorstack.js Changeset: 682889823712 Author:jlaskey Date: 2013-06-26 08:36 -0300 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/682889823712 8008458: Strict functions dont share property map Reviewed-by: sundar, hannesw Contributed-by: james.las...@oracle.com ! src/jdk/nashorn/internal/objects/NativeStrictArguments.java ! src/jdk/nashorn/internal/objects/ScriptFunctionImpl.java ! src/jdk/nashorn/internal/runtime/FindProperty.java ! src/jdk/nashorn/internal/runtime/Property.java ! src/jdk/nashorn/internal/runtime/PropertyMap.java ! src/jdk/nashorn/internal/runtime/ScriptObject.java ! src/jdk/nashorn/internal/runtime/SetMethodCreator.java ! src/jdk/nashorn/internal/runtime/UserAccessorProperty.java Changeset: 80c66d3fd872 Author:hannesw Date: 2013-06-26 15:40 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/80c66d3fd872 8019157: Avoid calling ScriptObject.setProto() if possible Reviewed-by: jlaskey, sundar ! buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ClassGenerator.java ! buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInstrumentor.java ! src/jdk/nashorn/internal/codegen/ClassEmitter.java ! src/jdk/nashorn/internal/codegen/CodeGenerator.java ! src/jdk/nashorn/internal/codegen/Compiler.java ! src/jdk/nashorn/internal/codegen/ObjectClassGenerator.java ! src/jdk/nashorn/internal/objects/AccessorPropertyDescriptor.java ! src/jdk/nashorn/internal/objects/ArrayBufferView.java ! src/jdk/nashorn/internal/objects/DataPropertyDescriptor.java ! src/jdk/nashorn/internal/objects/GenericPropertyDescriptor.java ! src/jdk/nashorn/internal/objects/Global.java ! src/jdk/nashorn/internal/objects/NativeArguments.java ! src/jdk/nashorn/internal/objects/NativeArray.java ! src/jdk/nashorn/internal/objects/NativeArrayBuffer.java ! src/jdk/nashorn/internal/objects/NativeBoolean.java ! src/jdk/nashorn/internal/objects/NativeDate.java ! src/jdk/nashorn/internal/objects/NativeDebug.java ! src/jdk/nashorn/internal/objects/NativeError.java ! src/jdk/nashorn/internal/objects/NativeEvalError.java ! src/jdk/nashorn/internal/objects/NativeFloat32Array.java ! src/jdk/nashorn/internal/objects/NativeFloat64Array.java ! src/jdk/nashorn/internal/objects/NativeFunction.java ! src/jdk/nashorn/internal/objects/NativeInt16Array.java ! src/jdk/nashorn/internal/objects/NativeInt32Array.java ! src/jdk/nashorn/internal/objects/NativeInt8Array.java ! src/jdk/nashorn/internal/objects/NativeJSAdapter.java !
hg: jdk8/tl/langtools: 8017609: javac, ClassFile.read(Path) should be ClassFile.read(Path, Attribute.Factory)
Changeset: 8e3d391c88c6 Author:vromero Date: 2013-06-27 09:54 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/8e3d391c88c6 8017609: javac, ClassFile.read(Path) should be ClassFile.read(Path, Attribute.Factory) Reviewed-by: jjg ! src/share/classes/com/sun/tools/classfile/ClassFile.java
hg: jdk8/tl/langtools: 8014513: Sjavac doesn't detect 32-bit jvm properly
Changeset: dcc6a52bf363 Author:erikj Date: 2013-06-27 10:35 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/dcc6a52bf363 8014513: Sjavac doesn't detect 32-bit jvm properly Reviewed-by: jjg ! src/share/classes/com/sun/tools/sjavac/CompileJavaPackages.java
Re: Review Request: JDK-8019227: JDK-8010325 broke the old build
On 27/06/2013 07:13, Alan Bateman wrote: On 27/06/2013 05:37, Brad Wetmore wrote: Brent/Alan/Mike, Hashing.java was removed from the JDK workspace, but was not removed from the old java/java/FILES_java.gmk. Things that still depend on the old build (JCE/deploy) are currently broken. http://cr.openjdk.java.net/~wetmore/8019227/webrev.00/ Brad P.S. I'm very aware that we need to move off the old build soon and am getting closer to finally working on it with Erik, and that the old build isn't complete. But it's complete enough for the JCE dependencies. Unfortunately, this isn't a simple transition (JDK-8006350), and this is a quick fix to get the JCE bits built. The old build has not produced usable bits for several months, it may not have been failing but if you look closely (or run the tests) then you'll see that there are several things missing. On build-dev then you'll probably have seen a mail or two from me where I was trying to encourage the build group to set a date to finally retire and remove the old build - this is because the old build is just a tax on every change that needs to touch the build. Anyway, the change looks okay but I strongly encourage you to put in the time to resolve the usability or others ssues that arise when working on JCE. +1 ( on the actual changes, and moving off the old build ) -Chris. -Alan
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
Sorry for high frequency of messages. This one is even better. powerCacheIndex need not be holding Nodes, but BigIntegers instead. So no indirection on fast-path: private static class Node { final BigInteger value; Node next; Node(BigInteger value) { this.value = value; } } private static final Node[] powerCache; private static final BigInteger[][] powerCacheIndex; private static final int POWER_CACHE_LINE_CHUNK = 16; static { powerCache = new Node[Character.MAX_RADIX + 1]; for (int i = Character.MIN_RADIX; i = Character.MAX_RADIX; i++) { powerCache[i] = new Node(BigInteger.valueOf(i)); } powerCacheIndex = new BigInteger[Character.MAX_RADIX + 1][]; } private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCacheIndex[radix]; if (cacheLine != null exponent cacheLine.length) { // cache line is long enough BigInteger value = cacheLine[exponent]; if (value != null) { return value; } return fillCacheLine(cacheLine, powerCache[radix], exponent); } else { // we need to extend / create cache line cacheLine = new BigInteger[(exponent / POWER_CACHE_LINE_CHUNK + 1) * POWER_CACHE_LINE_CHUNK]; BigInteger result = fillCacheLine(cacheLine, powerCache[radix], exponent); powerCacheIndex[radix] = cacheLine; // install new line return result; } } private static BigInteger fillCacheLine(BigInteger[] cacheLine, Node node, int exponent) { cacheLine[0] = node.value; for (int i = 1; i = exponent; i++) { synchronized (node) { if (node.next == null) { node.next = new Node(node.value.pow(2)); } } node = node.next; cacheLine[i] = node.value; } return node.value; } Regards, Peter On 06/27/2013 10:51 AM, Peter Levart wrote: Hi, Expanding on the idea of building single growing linked list of values and treating the array just as index for quick access which can be re-created at any time without synchronization or volatile publication, here's the attempt: private static class Node { final BigInteger value; Node next; Node(BigInteger value) { this.value = value; } } private static final Node[] powerCache; private static final Node[][] powerCacheIndex; private static final int POWER_CACHE_LINE_CHUNK = 16; static { powerCache = new Node[Character.MAX_RADIX + 1]; for (int i = Character.MIN_RADIX; i = Character.MAX_RADIX; i++) { powerCache[i] = new Node(BigInteger.valueOf(i)); } powerCacheIndex = new Node[Character.MAX_RADIX + 1][]; } private static BigInteger getRadixConversionCache(int radix, int exponent) { Node[] cacheLine = powerCacheIndex[radix]; if (cacheLine != null exponent cacheLine.length) { // cache line is long enough Node node = cacheLine[exponent]; if (node != null) { return node.value; } return fillCacheLine(cacheLine, powerCache[radix], exponent); } else { // we need to extend / create cache line cacheLine = new Node[(exponent / POWER_CACHE_LINE_CHUNK + 1) * POWER_CACHE_LINE_CHUNK]; BigInteger result = fillCacheLine(cacheLine, powerCache[radix], exponent); powerCacheIndex[radix] = cacheLine; // install new line return result; } } private static BigInteger fillCacheLine(Node[] cacheLine, Node node0, int exponent) { Node node = cacheLine[0] = node0; for (int i = 1; i = exponent; i++) { Node nextNode; synchronized (node) { nextNode = node.next; if (nextNode == null) { nextNode = new Node(node.value.square()); node.next = nextNode; } } cacheLine[i] = node = nextNode; } return node.value; } What do you think? Regards, Peter On 06/27/2013 09:27 AM, Peter Levart wrote: On 06/27/2013 08:37 AM, Peter Levart wrote: Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again
RFR 8017329 8b92-lambda regression: TreeSet(a, b).stream().substream(1).parallel().iterator() is empty
Hi, Please review a fix for a regression with using Stream.substream for an ORDERED but not SUBSIZED spliterator that does not split. This is based off: http://cr.openjdk.java.net/~psandoz/tl/JDK-8012987-slice/webrev/ which is blocked waiting for the Comparator API webrev to go into tl, which should be soon (waiting on CCC approval). Paul. # HG changeset patch # User psandoz # Date 1372324256 -7200 # Node ID 3459ac0d695ffdf2f5120e63a9a4bb79a0af40dc # Parent d4cd5e16ac8f279e05974cdb2f6281c5f24e484c 8017329: 8b92-lambda regression: TreeSet(a, b).stream().substream(1).parallel().iterator() is empty Reviewed-by: diff -r d4cd5e16ac8f -r 3459ac0d695f src/share/classes/java/util/stream/SliceOps.java --- a/src/share/classes/java/util/stream/SliceOps.java Thu Jun 27 11:06:17 2013 +0200 +++ b/src/share/classes/java/util/stream/SliceOps.java Thu Jun 27 11:10:56 2013 +0200 @@ -598,9 +598,9 @@ final Node.BuilderP_OUT nb = op.makeNodeBuilder(sizeIfKnown, generator); SinkP_OUT opSink = op.opWrapSink(helper.getStreamAndOpFlags(), nb); helper.copyIntoWithCancel(helper.wrapSink(opSink), spliterator); -// It is necessary to truncate here since the result at the root -// can only be set once -return doTruncate(nb.build()); +// There is no need to truncate since the op performs the +// skipping and limiting of elements +return nb.build(); } else { NodeP_OUT node = helper.wrapAndCopyInto(helper.makeNodeBuilder(-1, generator), diff -r d4cd5e16ac8f -r 3459ac0d695f test/java/util/stream/test/org/openjdk/tests/java/util/stream/SliceOpTest.java --- a/test/java/util/stream/test/org/openjdk/tests/java/util/stream/SliceOpTest.java Thu Jun 27 11:06:17 2013 +0200 +++ b/test/java/util/stream/test/org/openjdk/tests/java/util/stream/SliceOpTest.java Thu Jun 27 11:10:56 2013 +0200 @@ -26,13 +26,16 @@ import java.util.*; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.DoubleStream; import java.util.stream.IntStream; import java.util.stream.LambdaTestHelpers; import java.util.stream.LongStream; import java.util.stream.OpTestCase; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import java.util.stream.StreamTestDataProvider; import java.util.stream.TestData; @@ -192,6 +195,53 @@ } } +public void testSkipLimitOpsWithNonSplittingSpliterator() { +class NonSplittingNotSubsizedOrderedSpliteratorT implements SpliteratorT { +SpliteratorT s; + +NonSplittingNotSubsizedOrderedSpliterator(SpliteratorT s) { +assert s.hasCharacteristics(Spliterator.ORDERED); +this.s = s; +} + +@Override +public boolean tryAdvance(Consumer? super T action) { +return s.tryAdvance(action); +} + +@Override +public void forEachRemaining(Consumer? super T action) { +s.forEachRemaining(action); +} + +@Override +public SpliteratorT trySplit() { +return null; +} + +@Override +public long estimateSize() { +return s.estimateSize(); +} + +@Override +public int characteristics() { +return s.characteristics() ~(Spliterator.SUBSIZED); +} + +@Override +public Comparator? super T getComparator() { +return s.getComparator(); +} +} +ListInteger list = IntStream.range(0, 100).boxed().collect(Collectors.toList()); +TestData.OfRefInteger data = TestData.Factory.ofSupplier( +Non splitting, not SUBSIZED, ORDERED, stream, +() - StreamSupport.stream(new NonSplittingNotSubsizedOrderedSpliterator(list.spliterator(; + +testSkipLimitOps(testSkipLimitOpsWithNonSplittingSpliterator, data); +} + @Test(dataProvider = StreamTestDataInteger, dataProviderClass = StreamTestDataProvider.class) public void testLimitOps(String name, TestData.OfRefInteger data) { ListInteger limits = sizes(data.size());
hg: jdk8/tl/jdk: 2 new changesets
Changeset: 370e7beff8a0 Author:wetmore Date: 2013-06-27 10:19 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/370e7beff8a0 8019227: JDK-8010325 broke the old build Reviewed-by: alanb, chegar ! make/java/java/FILES_java.gmk Changeset: 4e69a7dfbeac Author:chegar Date: 2013-06-27 10:21 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4e69a7dfbeac Merge
JVM application shutdown hooks
Dear members, I have a problem within an external library using one JVM Shutdown hook to perform resource cleanup (socket, lock file deletion ...) that seems buggy in java web start environment: sometimes the JVM reports an IllegalStateException happenning during that shutdown hook. This library uses java.util.logging to log warnings and exceptions but none is logged (console or java trace files). The 'lost' log messages is related to the LogManager's shutdown hook: // This private class is used as a shutdown hook. // It does a reset to close all open handlers. private class Cleaner extends Thread { private Cleaner() { /* Set context class loader to null in order to avoid * keeping a strong reference to an application classloader. */ this.setContextClassLoader(null); } public void run() { // This is to ensure the LogManager.clinit is completed // before synchronized block. Otherwise deadlocks are possible. LogManager mgr = manager; // If the global handlers haven't been initialized yet, we // don't want to initialize them just so we can close them! synchronized (LogManager.this) { // Note that death is imminent. deathImminent = true; initializedGlobalHandlers = true; } // Do a reset to close all active handlers. reset(); } } Without any log handler, the log messages are lost during shutdown ! I think it is annoying as it avoids me getting log and exceptions ... FYI, the ApplicationShutdownHooks class executes all 'application' hooks concurrently: static void runHooks() { CollectionThread threads; synchronized(ApplicationShutdownHooks.class) { threads = hooks.keySet(); hooks = null; } for (Thread hook : threads) { hook.start(); } for (Thread hook : threads) { try { hook.join(); } catch (InterruptedException x) { } } } So it is not possible to define hook priorities ... However, I looked at the JVM Shutdown class which supports hook priorities and I think that the J.U.L Cleaner hook should run after all application hooks. Here are the current Shutdown priorities: // The system shutdown hooks are registered with a predefined slot. // The list of shutdown hooks is as follows: // (0) Console restore hook // (1) Application hooks // (2) DeleteOnExit hook Moreover, as a RFE, it could be useful for applications to be able to define hook priorities within an application: I did that several times registering only 1 shutdown hook and handling correct ordering on my own ... but could be supported by the JDK itself. Finally, here are (below) the Open JDK8 usages of Runtime.addShutdownHook [19 occurrences] which are more system hooks than application hooks: com.sun.imageio.stream StreamCloser.java StreamCloser addToQueue anonymous java.security.PrivilegedAction run 101: Runtime.getRuntime().addShutdownHook(streamCloser); java.util.logging LogManager.java LogManager LogManager 255: Runtime.getRuntime().addShutdownHook(new Cleaner()); sun.font SunFontManager.java SunFontManager createFont2D anonymous java.security.PrivilegedAction run 2538: Runtime.getRuntime().addShutdownHook(fileCloser); sun.rmi.server Activation.java Activation init 240: Runtime.getRuntime().addShutdownHook(shutdownHook); sun.tools.jstat sun.awt.shell Win32ShellFolderManager2.java Win32ShellFolderManager2 ComInvoker ComInvoker anonymous java.security.PrivilegedActionjava.lang.Void run 493: Runtime.getRuntime().addShutdownHook( sun.awt.windows WToolkit.java WToolkit registerShutdownHook anonymous java.security.PrivilegedActionjava.lang.Void run 285: Runtime.getRuntime().addShutdownHook(shutdown); sun.java2d.d3d D3DScreenUpdateManager.java D3DScreenUpdateManager D3DScreenUpdateManager anonymous java.security.PrivilegedAction run 112: Runtime.getRuntime().addShutdownHook(shutdown); java.util.prefs FileSystemPreferences.java FileSystemPreferences anonymous java.security.PrivilegedActionjava.lang.Void run 439: Runtime.getRuntime().addShutdownHook(new Thread() { sun.awt.X11 XToolkit.java XToolkit init anonymous java.security.PrivilegedActionjava.lang.Void run 350: Runtime.getRuntime().addShutdownHook(shutdownThread); sun.awt X11GraphicsDevice.java X11GraphicsDevice setDisplayMode anonymous java.security.PrivilegedActionjava.lang.Void run 445: Runtime.getRuntime().addShutdownHook(t); java.util.prefs MacOSXPreferencesFile.java MacOSXPreferencesFile timer 356: Runtime.getRuntime().addShutdownHook(flushThread); sun.font CFontManager.java CFontManager createFont2D anonymous java.security.PrivilegedActionjava.lang.Object run 232: Runtime.getRuntime().addShutdownHook(fileCloser); sun.lwawt LWToolkit.java
Re: test with a 3rd party jar file?
On 27/06/2013 08:50, huizhe wang wrote: Hi Chris, Thanks for the information! I'll give it a try. On 6/26/2013 6:18 AM, Chris Hegarty wrote: The streams package recently added tests for exercising package-private implementation. Top level dir contain the TEST.properties to add to the bootclasspath: Do you mean TEST.properties need to be in the top level directory (e.g. javax/xml/jaxp), can it be put anywhere that contains a block of tests? Yes, I believe so. You must put it somewhere that it does not impact on other tests, since all tests below that directory hierarchy will be impacted by it. In my particular case, I only want to run a single test with the jar file on the bootclasspath. Create a directory structure somewhere in the regression library appropriate to your changes, say javax/xml/jaxp/boottest/. Create javax/xml/jaxp/boottest/TEST.properties. Then put your test at javax/xml/jaxp/boottest/xxx/yyy/zzz/Test.java ( where xxx.yyy.zzz is your package name ). -Chris. Thanks, Joe http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/ Example, individual test, whose test is in the java.util.stream package: http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/java/util/stream/DoubleNodeTest.java -Chris. On 25/06/2013 01:51, huizhe wang wrote: Thanks Sean and Rob. Yes, I was told before to avoid shell scripts in tests. I'll wait till after your investigation to see what's the best to do in this case. -Joe On 6/24/2013 3:42 PM, Rob McKenna wrote: Some interesting conversations were had lately about shell scripts during Joe Darcy's recent infrastructure tech talk. In particular around the idea of a jdk.testing package to provide libraries that would help with the types of operations seen in shell scripts. I'm really hoping to spend some time on this myself over the coming weeks. (in an effort to at least understand why we need shell scripts and whether we could do something else instead) In any case it should be possible to simply replace the script with a java program that does the same thing. That would require fiddling with Process however, and its debatable as to whether that would result in fewer test failures. (shell scripts counting for a proportionally large number of those failures) JSR199 might help to reduce the amount of ProcessBuilders required in this instance. -Rob
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
On 06/27/2013 10:37 AM, Peter Levart wrote: Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; } Yes, I'm voting for this one. All other improvements Peter had suggested, while interesting, seem to not to worth the trouble. -Aleksey.
hg: jdk8/tl/langtools: 7008643: inlined finally clauses confuse debuggers
Changeset: d137ce373c4c Author:vromero Date: 2013-06-27 16:06 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/d137ce373c4c 7008643: inlined finally clauses confuse debuggers Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/jvm/Gen.java + test/tools/javac/T7008643/InlinedFinallyConfuseDebuggersTest.java
hg: jdk8/tl/langtools: 8016099: Some @SuppressWarnings annotations ignored ( unchecked, rawtypes )
Changeset: e42c27026290 Author:vromero Date: 2013-06-27 16:04 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/e42c27026290 8016099: Some @SuppressWarnings annotations ignored ( unchecked, rawtypes ) Reviewed-by: jjg ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! src/share/classes/com/sun/tools/javac/comp/Check.java + test/tools/javac/T8016099/UncheckedWarningRegressionTest.java + test/tools/javac/T8016099/UncheckedWarningRegressionTest.out
RFR 8011427 java.util.concurrent collection Spliterator implementations
Hi, This webrev contains updates of java.util.concurrent collections from 166: http://cr.openjdk.java.net/~psandoz/tl/JDK-8011427-concur-splits/webrev/ More specifically: - spliterator implementations (which are tested by the existing spltierator tests). - various doc updates. - and other updates that are tricky to separate from the above as continual improvements are made, especially to CopyOnWriteArrayList and ConcurrentSkipListMap. ConcurrentHashMap, ConcurrentMap and ConcurrentNavigableMap will be dealt with in a separate webrev, as will other aspects of 166. Most of this code has been in the lambda repo for quite a while (the diff between 166 and lambda is much much smaller than the diff between tl and lambda). This webrev was derived from the lambda code base after first syncing lambda with associated files from 166. Paul.
Re: RFR 8011427 java.util.concurrent collection Spliterator implementations
I forgot to mention this patch is based off the comparator's patch which is required by updates to ConcurrentSkipListMap.java. Paul. On Jun 27, 2013, at 5:33 PM, Paul Sandoz paul.san...@oracle.com wrote: Hi, This webrev contains updates of java.util.concurrent collections from 166: http://cr.openjdk.java.net/~psandoz/tl/JDK-8011427-concur-splits/webrev/ More specifically: - spliterator implementations (which are tested by the existing spltierator tests). - various doc updates. - and other updates that are tricky to separate from the above as continual improvements are made, especially to CopyOnWriteArrayList and ConcurrentSkipListMap. ConcurrentHashMap, ConcurrentMap and ConcurrentNavigableMap will be dealt with in a separate webrev, as will other aspects of 166. Most of this code has been in the lambda repo for quite a while (the diff between 166 and lambda is much much smaller than the diff between tl and lambda). This webrev was derived from the lambda code base after first syncing lambda with associated files from 166. Paul.
hg: jdk8/tl/langtools: 8015720: since tag isn't copied while generating JavaFX documentation
Changeset: 26437287529d Author:janvalenta Date: 2013-06-27 17:47 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/26437287529d 8015720: since tag isn't copied while generating JavaFX documentation Reviewed-by: jjg ! src/share/classes/com/sun/tools/doclets/internal/toolkit/builders/MemberSummaryBuilder.java ! src/share/classes/com/sun/tools/doclets/internal/toolkit/taglets/TagletManager.java ! test/com/sun/javadoc/testJavaFX/C.java ! test/com/sun/javadoc/testJavaFX/TestJavaFX.java
Re: test with a 3rd party jar file?
Thanks Chris for the instructions! Joe On 6/27/2013 3:34 AM, Chris Hegarty wrote: On 27/06/2013 08:50, huizhe wang wrote: Hi Chris, Thanks for the information! I'll give it a try. On 6/26/2013 6:18 AM, Chris Hegarty wrote: The streams package recently added tests for exercising package-private implementation. Top level dir contain the TEST.properties to add to the bootclasspath: Do you mean TEST.properties need to be in the top level directory (e.g. javax/xml/jaxp), can it be put anywhere that contains a block of tests? Yes, I believe so. You must put it somewhere that it does not impact on other tests, since all tests below that directory hierarchy will be impacted by it. In my particular case, I only want to run a single test with the jar file on the bootclasspath. Create a directory structure somewhere in the regression library appropriate to your changes, say javax/xml/jaxp/boottest/. Create javax/xml/jaxp/boottest/TEST.properties. Then put your test at javax/xml/jaxp/boottest/xxx/yyy/zzz/Test.java ( where xxx.yyy.zzz is your package name ). -Chris. Thanks, Joe http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/ Example, individual test, whose test is in the java.util.stream package: http://hg.openjdk.java.net/jdk8/tl/jdk/file/510035b7/test/java/util/stream/boottest/java/util/stream/DoubleNodeTest.java -Chris. On 25/06/2013 01:51, huizhe wang wrote: Thanks Sean and Rob. Yes, I was told before to avoid shell scripts in tests. I'll wait till after your investigation to see what's the best to do in this case. -Joe On 6/24/2013 3:42 PM, Rob McKenna wrote: Some interesting conversations were had lately about shell scripts during Joe Darcy's recent infrastructure tech talk. In particular around the idea of a jdk.testing package to provide libraries that would help with the types of operations seen in shell scripts. I'm really hoping to spend some time on this myself over the coming weeks. (in an effort to at least understand why we need shell scripts and whether we could do something else instead) In any case it should be possible to simply replace the script with a java program that does the same thing. That would require fiddling with Process however, and its debatable as to whether that would result in fewer test failures. (shell scripts counting for a proportionally large number of those failures) JSR199 might help to reduce the amount of ProcessBuilders required in this instance. -Rob
RFR: 8019224 : add exception chaining to RMI CGIHandler
Hi all, Here's a simple webrev to add exception chaining (constructor overloads with the 'cause' parameter) to some internal exception types used in RMI. We've been getting some test failures that go through this area and I suspect that this code is discarding some diagnostic information. http://cr.openjdk.java.net/~smarks/reviews/8019224/webrev.0/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8019224 Thanks! s'marks
Re: RFR: 8019224 : add exception chaining to RMI CGIHandler
Looks fine Stuart; cheers, -Joe On 06/27/2013 09:21 AM, Stuart Marks wrote: Hi all, Here's a simple webrev to add exception chaining (constructor overloads with the 'cause' parameter) to some internal exception types used in RMI. We've been getting some test failures that go through this area and I suspect that this code is discarding some diagnostic information. http://cr.openjdk.java.net/~smarks/reviews/8019224/webrev.0/ http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8019224 Thanks! s'marks
Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java
If this is to be undone after the correct zip fix, why not add the @ignore for now ? and enable this when 8015666 is fixed correctly. Kumar This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman/8015666/webrev/ The proposal for #8015666 is to keep the existing behavior of ZipEntry.getTime() to return a LastModifiedTime converted from the zip entry's ms-dos-formatted date/time field by using the default timezone. A new pair ZipEntry.get/setLastModifiedTime() will be added to access the real UTC time stored in the zip entry, if presents. The API doc will be updated accordingly as well to explicitly explain the source of the date/time and the its timezone sensitive conversion. -Sherman On 06/25/2013 07:03 AM, Eric McCorkle wrote: Hello, Please review this simple patch which updates regression test langtools/tools/javac/T6725036.java to offset the time returned by JavaFileObject.getLastModified() with the local time to UTC delta. Please note that this patch is intended to address the test failures, and that I will be immediately opening a new bug to investigate and address deeper issues, and also to properly document the API. The webrev is here: http://cr.openjdk.java.net/~emc/8016760/ The bug report is here: http://bugs.sun.com/view_bug.do?bug_id=8016760 Thanks, Eric
review request for bug: 8017471 clean up of JDBC doclint errors
Hi all, Need a reviewer for 8017471 which addresses the JDBC doclint errors The webrev can be found at http://cr.openjdk.java.net/~lancea/8017471/webrev.00/ please note that for the WebRowset changes, you will need to use the udiff vs the sdiff formant as webrev generates errors due to the number of changes: -- src/share/classes/javax/sql/rowset/WebRowSet.java patch cdiffs udiffs/usr/bin/awk: limited to 50 pat,pat statements at source line 74 source file /tmp/64799.file1 context is NR==374,NR==377 {changed();next} /usr/bin/awk: limited to 50 pat,pat statements at source line 75 source file /tmp/64799.file1 /usr/bin/awk: limited to 50 pat,pat statements at source line 76 source file /tmp/64799.file1 /usr/bin/awk: limited to 50 pat,pat statements at source line 75 source file /tmp/64799.file2 -- Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing
Hi Sherman, I started looking at this, my initial comment, the Unpacker.unpack does not close its output and we allow multiple pack files to be concatenated, I am assuming out.finish() will allow further jar files to be appended ? or would this cause a problem ? Kumar Hi, The zip time related changes[1] I pushed back last month appears to have the compatibility risk of breaking existing code. The main idea in that changeset is to use the more accurate and timezone insensitive utc time stored in the extra field for the ZipEntry.set/getTime() if possible. However it turns out the reality is that the code out there might have already had some interesting workaround/hack solution to workaround the problem that the time stamp stored in the standard' zip entry header is a MS-DOS standard date/time, which is a local date/time and sensitive to timezone, in which, if the zip is archived in time zone A (our implementation converts the java time to dos time by using the default tz A) and then transferred/un-archived in a different zone B (use default tz B to convert back to java time), we have a time stamp mess up. The workaround from pack200 for this issue when pack/unpacking a jar file is to specify/recommend/suggest in its spec that the time zone in a jar file entry is assumed to be UTC, so the pack/unpack200 implementation set the default time to utc before the pack/unpack and set it back to the original after that. It worked perfectly for a roundtrip pack/unpacking, until the changeset [2], in which ZipEntry.getTime() (packing) returns a real utc time and the following ZipEntry.setTime() (unpacking), then mess up the MS-DOS date/time entry, this is the root cause of this regression. Given the facts that (1) there are actually two real physical time stamps in a zip file header, one is in the date/time fields, which is MS-DOS-local-date/time-with-2- seconds-granularity , one is in the extra data field, which is UTC-1-second -granularity (2) and there are applications over there that have dependency on the MS-DOS date/time stamp. I'm proposing the following approach to add the functionality of supporting the utc-date/time-with-1-second granularity and keep the old behavior of the get/setTime() of the ZipEntry. (1) keep the time/setTime()/getTime() for the MS-DOS standard date/time. To set via the old setTime() will only store the time into zip's standard date/time field, which is in MS-DOS date/time. And getTime() only returns the date/time from that field, when read from the zip file/stream. (2) add mtime/set/getLastModifiedTime() to work on the UTC time fields, and the last modified time set via the new method will also set the time, and the getLastModifiedTime() also returns the time, if the UTC time stamp fields are not set in the zip file header. The idea is that for the new application, the recommendation is to use ZipEntry.set/getLastModifiedTime() for better/correct time stamp, but the existing apps keep the same behavior. (3) jar and ZipOutputStream are updated to use the set/getLastModifiedTime(). (4) Pack/unpack continues to use the set/getTime(), so the current workaround continues work. I will leave this to Kuma to decide how it should be handled going forward. (there are two facts need to be considered here, a) the existing jar file might not have the utc time instored, and b) all extra data are wiped out during the pack/unpacking process) (5) additionally add another pair of atime/get/setLastAccessTime and ctime/get/setCreationTime(). (6) The newly added 3 pairs of the m/a/ctime get/set methods use the new nio FileTime, instead of the long. This may add some additional cost of conversion when working with them, but may also help improve the performance if the time stamps are directly from nio file system when get/set XYZTime. Good/bad? http://cr.openjdk.java.net/~sherman/8015666/webrev/ Comment, option and suggestion are appreciated. -Sherman [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90df6756406f
Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java
Yes, I think that this is the correct approach. -- Jon On 06/27/2013 09:43 AM, Kumar Srinivasan wrote: If this is to be undone after the correct zip fix, why not add the @ignore for now ? and enable this when 8015666 is fixed correctly. Kumar This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman/8015666/webrev/ The proposal for #8015666 is to keep the existing behavior of ZipEntry.getTime() to return a LastModifiedTime converted from the zip entry's ms-dos-formatted date/time field by using the default timezone. A new pair ZipEntry.get/setLastModifiedTime() will be added to access the real UTC time stored in the zip entry, if presents. The API doc will be updated accordingly as well to explicitly explain the source of the date/time and the its timezone sensitive conversion. -Sherman On 06/25/2013 07:03 AM, Eric McCorkle wrote: Hello, Please review this simple patch which updates regression test langtools/tools/javac/T6725036.java to offset the time returned by JavaFileObject.getLastModified() with the local time to UTC delta. Please note that this patch is intended to address the test failures, and that I will be immediately opening a new bug to investigate and address deeper issues, and also to properly document the API. The webrev is here: http://cr.openjdk.java.net/~emc/8016760/ The bug report is here: http://bugs.sun.com/view_bug.do?bug_id=8016760 Thanks, Eric
Re: RFR: 8015666: test/tools/pack200/TimeStamp.java failing
On 06/27/2013 10:04 AM, Kumar Srinivasan wrote: Hi Sherman, I started looking at this, my initial comment, the Unpacker.unpack does not close its output and we allow multiple pack files to be concatenated, I am assuming out.finish() will allow further jar files to be appended ? or would this cause a problem ? No, out.finish() will not allow further entry appending. Then, it appears we need to have a different approach to finish the Jar/ZipOutputStream. What need to be done here is that either out.close/finish() need to be invoked under the UTC locale as well (to output the time stamps in cen table correctly). This is another incompatible change of the previous change, in which the msdosTime-javaTime conversion no longer occurs during the ZipEntry.set/getTime(), but during the read and write the ZipEntry from/to the zip file. -Sherman Kumar Hi, The zip time related changes[1] I pushed back last month appears to have the compatibility risk of breaking existing code. The main idea in that changeset is to use the more accurate and timezone insensitive utc time stored in the extra field for the ZipEntry.set/getTime() if possible. However it turns out the reality is that the code out there might have already had some interesting workaround/hack solution to workaround the problem that the time stamp stored in the standard' zip entry header is a MS-DOS standard date/time, which is a local date/time and sensitive to timezone, in which, if the zip is archived in time zone A (our implementation converts the java time to dos time by using the default tz A) and then transferred/un-archived in a different zone B (use default tz B to convert back to java time), we have a time stamp mess up. The workaround from pack200 for this issue when pack/unpacking a jar file is to specify/recommend/suggest in its spec that the time zone in a jar file entry is assumed to be UTC, so the pack/unpack200 implementation set the default time to utc before the pack/unpack and set it back to the original after that. It worked perfectly for a roundtrip pack/unpacking, until the changeset [2], in which ZipEntry.getTime() (packing) returns a real utc time and the following ZipEntry.setTime() (unpacking), then mess up the MS-DOS date/time entry, this is the root cause of this regression. Given the facts that (1) there are actually two real physical time stamps in a zip file header, one is in the date/time fields, which is MS-DOS-local-date/time-with-2- seconds-granularity , one is in the extra data field, which is UTC-1-second -granularity (2) and there are applications over there that have dependency on the MS-DOS date/time stamp. I'm proposing the following approach to add the functionality of supporting the utc-date/time-with-1-second granularity and keep the old behavior of the get/setTime() of the ZipEntry. (1) keep the time/setTime()/getTime() for the MS-DOS standard date/time. To set via the old setTime() will only store the time into zip's standard date/time field, which is in MS-DOS date/time. And getTime() only returns the date/time from that field, when read from the zip file/stream. (2) add mtime/set/getLastModifiedTime() to work on the UTC time fields, and the last modified time set via the new method will also set the time, and the getLastModifiedTime() also returns the time, if the UTC time stamp fields are not set in the zip file header. The idea is that for the new application, the recommendation is to use ZipEntry.set/getLastModifiedTime() for better/correct time stamp, but the existing apps keep the same behavior. (3) jar and ZipOutputStream are updated to use the set/getLastModifiedTime(). (4) Pack/unpack continues to use the set/getTime(), so the current workaround continues work. I will leave this to Kuma to decide how it should be handled going forward. (there are two facts need to be considered here, a) the existing jar file might not have the utc time instored, and b) all extra data are wiped out during the pack/unpacking process) (5) additionally add another pair of atime/get/setLastAccessTime and ctime/get/setCreationTime(). (6) The newly added 3 pairs of the m/a/ctime get/set methods use the new nio FileTime, instead of the long. This may add some additional cost of conversion when working with them, but may also help improve the performance if the time stamps are directly from nio file system when get/set XYZTime. Good/bad? http://cr.openjdk.java.net/~sherman/8015666/webrev/ Comment, option and suggestion are appreciated. -Sherman [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/90df6756406f
Re: review request for bug: 8017471 clean up of JDBC doclint errors
Hi Lance, I looked through the patch and it looks fine; thanks, -Joe On 06/27/2013 09:56 AM, Lance Andersen wrote: Hi all, Need a reviewer for 8017471 which addresses the JDBC doclint errors The webrev can be found at http://cr.openjdk.java.net/~lancea/8017471/webrev.00/ please note that for the WebRowset changes, you will need to use the udiff vs the sdiff formant as webrev generates errors due to the number of changes: -- src/share/classes/javax/sql/rowset/WebRowSet.java patch cdiffs udiffs/usr/bin/awk: limited to 50 pat,pat statements at source line 74 source file /tmp/64799.file1 context is NR==374,NR==377 {changed();next} /usr/bin/awk: limited to 50 pat,pat statements at source line 75 source file /tmp/64799.file1 /usr/bin/awk: limited to 50 pat,pat statements at source line 76 source file /tmp/64799.file1 /usr/bin/awk: limited to 50 pat,pat statements at source line 75 source file /tmp/64799.file2 -- Best Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: JDK 8 request for review: doclint cleanup of java.util.prefs
+1 On Jun 27, 2013, at 1:59 PM, Joe Darcy joe.da...@oracle.com wrote: Hello, Please review the patch below which resolves doclint warnings in java.util.prefs. Thanks, -Joe diff -r 4e69a7dfbeac src/share/classes/java/util/prefs/AbstractPreferences.java --- a/src/share/classes/java/util/prefs/AbstractPreferences.java Thu Jun 27 10:21:22 2013 +0100 +++ b/src/share/classes/java/util/prefs/AbstractPreferences.java Thu Jun 27 10:57:21 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -1121,6 +1121,8 @@ * removed. (The implementor needn't check for any of these things.) * * pThis method is invoked with the lock on this node held. + * @param key the key + * @param value the value */ protected abstract void putSpi(String key, String value); @@ -1139,6 +1141,7 @@ * * pThis method is invoked with the lock on this node held. * + * @param key the key * @return the value associated with the specified key at this preference * node, or ttnull/tt if there is no association for this * key, or the association cannot be determined at this time. @@ -1152,6 +1155,7 @@ * (The implementor needn't check for either of these things.) * * pThis method is invoked with the lock on this node held. + * @param key the key */ protected abstract void removeSpi(String key); diff -r 4e69a7dfbeac src/share/classes/java/util/prefs/PreferencesFactory.java --- a/src/share/classes/java/util/prefs/PreferencesFactory.java Thu Jun 27 10:21:22 2013 +0100 +++ b/src/share/classes/java/util/prefs/PreferencesFactory.java Thu Jun 27 10:57:21 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -45,6 +45,7 @@ /** * Returns the system root preference node. (Multiple calls on this * method will return the same object reference.) + * @return the system root preference node */ Preferences systemRoot(); @@ -52,6 +53,8 @@ * Returns the user root preference node corresponding to the calling * user. In a server, the returned value will typically depend on * some implicit client-context. + * @return the user root preference node corresponding to the calling + * user */ Preferences userRoot(); } Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
hg: jdk8/tl/jdk: 8019304: Fix doclint issues in java.util.prefs
Changeset: 1c31082f0a51 Author:darcy Date: 2013-06-27 11:06 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1c31082f0a51 8019304: Fix doclint issues in java.util.prefs Reviewed-by: lancea ! src/share/classes/java/util/prefs/AbstractPreferences.java ! src/share/classes/java/util/prefs/PreferencesFactory.java
JDK 8 request for review: doclint cleanup of java.util.prefs
Hello, Please review the patch below which resolves doclint warnings in java.util.prefs. Thanks, -Joe diff -r 4e69a7dfbeac src/share/classes/java/util/prefs/AbstractPreferences.java --- a/src/share/classes/java/util/prefs/AbstractPreferences.java Thu Jun 27 10:21:22 2013 +0100 +++ b/src/share/classes/java/util/prefs/AbstractPreferences.java Thu Jun 27 10:57:21 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -1121,6 +1121,8 @@ * removed. (The implementor needn't check for any of these things.) * * pThis method is invoked with the lock on this node held. + * @param key the key + * @param value the value */ protected abstract void putSpi(String key, String value); @@ -1139,6 +1141,7 @@ * * pThis method is invoked with the lock on this node held. * + * @param key the key * @return the value associated with the specified key at this preference * node, or ttnull/tt if there is no association for this * key, or the association cannot be determined at this time. @@ -1152,6 +1155,7 @@ * (The implementor needn't check for either of these things.) * * pThis method is invoked with the lock on this node held. + * @param key the key */ protected abstract void removeSpi(String key); diff -r 4e69a7dfbeac src/share/classes/java/util/prefs/PreferencesFactory.java --- a/src/share/classes/java/util/prefs/PreferencesFactory.java Thu Jun 27 10:21:22 2013 +0100 +++ b/src/share/classes/java/util/prefs/PreferencesFactory.java Thu Jun 27 10:57:21 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -45,6 +45,7 @@ /** * Returns the system root preference node. (Multiple calls on this * method will return the same object reference.) + * @return the system root preference node */ Preferences systemRoot(); @@ -52,6 +53,8 @@ * Returns the user root preference node corresponding to the calling * user. In a server, the returned value will typically depend on * some implicit client-context. + * @return the user root preference node corresponding to the calling + * user */ Preferences userRoot(); }
Re: JDK 8 request for review: doclint cleanup of java.util.prefs
On 27/06/2013 18:59, Joe Darcy wrote: Hello, Please review the patch below which resolves doclint warnings in java.util.prefs. Thanks, -Joe This looks okay to me although you might want to put in a blank line before the @param list so that is consistent with the other prefs classes. -Alan.
Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java
Accidentally dropped core-libs-dev from the reply. On 06/27/13 13:31, Eric McCorkle wrote: I could easily replace the patch I have now with one that just marks the test @ignore and commit it. Do we want to go ahead and approve that? On 06/27/13 13:12, Jonathan Gibbons wrote: Yes, I think that this is the correct approach. -- Jon On 06/27/2013 09:43 AM, Kumar Srinivasan wrote: If this is to be undone after the correct zip fix, why not add the @ignore for now ? and enable this when 8015666 is fixed correctly. Kumar This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman/8015666/webrev/ The proposal for #8015666 is to keep the existing behavior of ZipEntry.getTime() to return a LastModifiedTime converted from the zip entry's ms-dos-formatted date/time field by using the default timezone. A new pair ZipEntry.get/setLastModifiedTime() will be added to access the real UTC time stored in the zip entry, if presents. The API doc will be updated accordingly as well to explicitly explain the source of the date/time and the its timezone sensitive conversion. -Sherman On 06/25/2013 07:03 AM, Eric McCorkle wrote: Hello, Please review this simple patch which updates regression test langtools/tools/javac/T6725036.java to offset the time returned by JavaFileObject.getLastModified() with the local time to UTC delta. Please note that this patch is intended to address the test failures, and that I will be immediately opening a new bug to investigate and address deeper issues, and also to properly document the API. The webrev is here: http://cr.openjdk.java.net/~emc/8016760/ The bug report is here: http://bugs.sun.com/view_bug.do?bug_id=8016760 Thanks, Eric
Re: RFC: 6178739 - Formatter - Zero padding flag with zero width
On Jun 26, 2013, at 10:55 PM, David DeHaven wrote: Specifically, I was referred to how C handles %0.4f\n. No width, decimal truncated (rounded? floored? I forgot which it is) to four digits. -DrD- printf(%0.4f\n, 56789.456789F); ... 56789.4570 ^ ^ ^ ^ ^ ^ ^ ^ ... A leading zero in the width value is interpreted as the zero-padding flag mentioned above […]. Only if there's a valid width following, which there isn't in the case above. Try %016.4 with the above test. Note that the width is the *full* width of the entire field, including decimal point and following digits. printf(%016.4f\n, 56789.456789F); printf(%0.4f\n, 56789.456789F); Produces: 0056789.4570 56789.4570 printf(%016.4f\n, 56789.456789F); printf(%1.4f\n, 56789.456789F); printf(%0.4f\n, 56789.456789F); produces (on Mac OS X) 0056789.4570 56789.4570 56789.4570 whereas System.out.printf(%016.4f\n, 56789.456789); System.out.printf(%1.4f\n, 56789.456789); System.out.printf(%0.4f\n, 56789.456789); produces 0056789.4568 56789.4568 Exception in thread main java.util.MissingFormatWidthException: %0.4f Two possible solutions are: 1) Change the specification of the width field to The optional width is a positive decimal integer indicating the minimum number of characters to be written to the output. A leading zero in the width value is interpreted to be the zero-padding flag discussed below. 2) Handle a 0.x as indicating zero width, not zero padding. Brian
Re: RFR 8017540: Improve multi-threaded contention behavior of BigInteger.toString() radix conversion cache
On Jun 27, 2013, at 4:18 AM, Aleksey Shipilev wrote: On 06/27/2013 10:37 AM, Peter Levart wrote: Hi, This version seems correct. Maybe just replace double volatile read at length re-check with a single one: private static BigInteger getRadixConversionCache(int radix, int exponent) { BigInteger[] cacheLine = powerCache[radix]; // volatile read if (exponent cacheLine.length) return cacheLine[exponent]; int oldLength = cacheLine.length; cacheLine = Arrays.copyOf(cacheLine, exponent + 1); for (int i = oldLength; i = exponent; i++) cacheLine[i] = cacheLine[i - 1].pow(2); BigInteger[][] pc = powerCache; // volatile read again if (exponent = pc[radix].length) { pc = pc.clone(); pc[radix] = cacheLine; powerCache = pc; // volatile write, publish } return cacheLine[exponent]; } Yes, I'm voting for this one. All other improvements Peter had suggested, while interesting, seem to not to worth the trouble. Are there any objections to this version (as included above)? Brian
hg: jdk8/tl/langtools: 8019308: Add descriptions of Java SE 7 and 8 language changes to SourceVersion
Changeset: 065f8cb7bd89 Author:darcy Date: 2013-06-27 11:46 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/065f8cb7bd89 8019308: Add descriptions of Java SE 7 and 8 language changes to SourceVersion Reviewed-by: jjg ! src/share/classes/javax/lang/model/SourceVersion.java
JDK 8 code review request: doclint cleanup of java.util.logging
Hello, Please review the patch below which resolves doclint warnings in java.util.logging. Thanks, -Joe diff -r 1c31082f0a51 src/share/classes/java/util/logging/Handler.java --- a/src/share/classes/java/util/logging/Handler.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/Handler.javaThu Jun 27 11:56:23 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -209,6 +209,7 @@ /** * Retrieves the ErrorManager for this Handler. * + * @return the ErrorManager for this Handler * @exception SecurityException if a security manager exists and if * the caller does not have ttLoggingPermission(control)/tt. */ diff -r 1c31082f0a51 src/share/classes/java/util/logging/LogManager.java --- a/src/share/classes/java/util/logging/LogManager.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/LogManager.javaThu Jun 27 11:56:23 2013 -0700 @@ -257,7 +257,8 @@ } /** - * Return the global LogManager object. + * Returns the global LogManager object. + * @return the global LogManager object */ public static LogManager getLogManager() { if (manager != null) { diff -r 1c31082f0a51 src/share/classes/java/util/logging/LogRecord.java --- a/src/share/classes/java/util/logging/LogRecord.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/LogRecord.javaThu Jun 27 11:56:23 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -211,6 +211,7 @@ * the message string before formatting it. The result may * be null if the message is not localizable, or if no suitable * ResourceBundle is available. + * @return the localization resource bundle */ public ResourceBundle getResourceBundle() { return resourceBundle; @@ -231,6 +232,7 @@ * This is the name for the ResourceBundle that should be * used to localize the message string before formatting it. * The result may be null if the message is not localizable. + * @return the localization resource bundle name */ public String getResourceBundleName() { return resourceBundleName; @@ -281,6 +283,7 @@ * p * Sequence numbers are normally assigned in the LogRecord constructor, * so it should not normally be necessary to use this method. + * @param seq the sequence number */ public void setSequenceNumber(long seq) { sequenceNumber = seq;
hg: jdk8/tl/jdk: 8017471: Fix JDBC -Xdoclint public errors
Changeset: b9ba04dc210f Author:lancea Date: 2013-06-27 15:07 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b9ba04dc210f 8017471: Fix JDBC -Xdoclint public errors Reviewed-by: darcy ! src/share/classes/java/sql/Blob.java ! src/share/classes/java/sql/CallableStatement.java ! src/share/classes/java/sql/Clob.java ! src/share/classes/java/sql/DatabaseMetaData.java ! src/share/classes/java/sql/Driver.java ! src/share/classes/java/sql/DriverAction.java ! src/share/classes/java/sql/NClob.java ! src/share/classes/java/sql/ResultSet.java ! src/share/classes/java/sql/SQLInput.java ! src/share/classes/java/sql/SQLPermission.java ! src/share/classes/java/sql/SQLXML.java ! src/share/classes/java/sql/Wrapper.java ! src/share/classes/javax/sql/CommonDataSource.java ! src/share/classes/javax/sql/ConnectionPoolDataSource.java ! src/share/classes/javax/sql/DataSource.java ! src/share/classes/javax/sql/RowSet.java ! src/share/classes/javax/sql/XADataSource.java ! src/share/classes/javax/sql/rowset/BaseRowSet.java ! src/share/classes/javax/sql/rowset/CachedRowSet.java ! src/share/classes/javax/sql/rowset/FilteredRowSet.java ! src/share/classes/javax/sql/rowset/JdbcRowSet.java ! src/share/classes/javax/sql/rowset/Joinable.java ! src/share/classes/javax/sql/rowset/Predicate.java ! src/share/classes/javax/sql/rowset/RowSetProvider.java ! src/share/classes/javax/sql/rowset/RowSetWarning.java ! src/share/classes/javax/sql/rowset/WebRowSet.java ! src/share/classes/javax/sql/rowset/package.html ! src/share/classes/javax/sql/rowset/serial/SerialArray.java ! src/share/classes/javax/sql/rowset/serial/SerialBlob.java ! src/share/classes/javax/sql/rowset/serial/SerialClob.java ! src/share/classes/javax/sql/rowset/serial/SerialDatalink.java ! src/share/classes/javax/sql/rowset/serial/SerialJavaObject.java ! src/share/classes/javax/sql/rowset/serial/SerialRef.java ! src/share/classes/javax/sql/rowset/serial/SerialStruct.java ! src/share/classes/javax/sql/rowset/spi/SyncFactory.java ! src/share/classes/javax/sql/rowset/spi/SyncResolver.java
Re: Review Request: JDK-8019227: JDK-8010325 broke the old build
The old build has not produced usable bits for several months, it may not have been failing but if you look closely (or run the tests) then you'll see that there are several things missing. On build-dev then you'll probably have seen a mail or two from me where I was trying to encourage the build group to set a date to finally retire and remove the old build - this is because the old build is just a tax on every change that needs to touch the build. Anyway, the change looks okay but I strongly encourage you to put in the time to resolve the usability or others ssues that arise when working on JCE. Alan, I tried to save you the effort of composing just such a reply. I've seen your mails and agree with your sentiment. You are preaching to the choir [1]. ;) Thank you Chris, for doing the operation! Brad [1] http://idioms.thefreedictionary.com/preach+to+the+choir
Re: JDK 8 code review request: doclint cleanup of java.util.logging
+1 On Jun 27, 2013, at 2:58 PM, Joe Darcy wrote: Hello, Please review the patch below which resolves doclint warnings in java.util.logging. Thanks, -Joe diff -r 1c31082f0a51 src/share/classes/java/util/logging/Handler.java --- a/src/share/classes/java/util/logging/Handler.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/Handler.javaThu Jun 27 11:56:23 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -209,6 +209,7 @@ /** * Retrieves the ErrorManager for this Handler. * + * @return the ErrorManager for this Handler * @exception SecurityException if a security manager exists and if * the caller does not have ttLoggingPermission(control)/tt. */ diff -r 1c31082f0a51 src/share/classes/java/util/logging/LogManager.java --- a/src/share/classes/java/util/logging/LogManager.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/LogManager.javaThu Jun 27 11:56:23 2013 -0700 @@ -257,7 +257,8 @@ } /** - * Return the global LogManager object. + * Returns the global LogManager object. + * @return the global LogManager object */ public static LogManager getLogManager() { if (manager != null) { diff -r 1c31082f0a51 src/share/classes/java/util/logging/LogRecord.java --- a/src/share/classes/java/util/logging/LogRecord.javaThu Jun 27 11:06:46 2013 -0700 +++ b/src/share/classes/java/util/logging/LogRecord.javaThu Jun 27 11:56:23 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -211,6 +211,7 @@ * the message string before formatting it. The result may * be null if the message is not localizable, or if no suitable * ResourceBundle is available. + * @return the localization resource bundle */ public ResourceBundle getResourceBundle() { return resourceBundle; @@ -231,6 +232,7 @@ * This is the name for the ResourceBundle that should be * used to localize the message string before formatting it. * The result may be null if the message is not localizable. + * @return the localization resource bundle name */ public String getResourceBundleName() { return resourceBundleName; @@ -281,6 +283,7 @@ * p * Sequence numbers are normally assigned in the LogRecord constructor, * so it should not normally be necessary to use this method. + * @param seq the sequence number */ public void setSequenceNumber(long seq) { sequenceNumber = seq; Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
hg: jdk8/tl/jdk: 8019315: Fix doclint issues in java.util.logging
Changeset: b8f16cb2d95b Author:darcy Date: 2013-06-27 12:24 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b8f16cb2d95b 8019315: Fix doclint issues in java.util.logging Reviewed-by: lancea ! src/share/classes/java/util/logging/Handler.java ! src/share/classes/java/util/logging/LogManager.java ! src/share/classes/java/util/logging/LogRecord.java
hg: jdk8/tl/langtools: 7080001: Need to bump version numbers in build.properties for 8
Changeset: 97e798c06804 Author:ksrini Date: 2013-06-27 12:42 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/97e798c06804 7080001: Need to bump version numbers in build.properties for 8 Reviewed-by: jjg ! make/build.properties
JDK 8 core review request for doclint warnings in javax.script
Hello, Please review the next patch in a series of patches to resolve doclint warnings in the JDK; this time in javax.script. Thanks, -Joe diff -r b8f16cb2d95b src/share/classes/javax/script/Invocable.java --- a/src/share/classes/javax/script/Invocable.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/Invocable.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -63,6 +63,7 @@ /** * Used to call top-level procedures and functions defined in scripts. * + * @param name of the procedure or function to call * @param args Arguments to pass to the procedure or function * @return The value returned by the procedure or function * @@ -79,6 +80,7 @@ * the interpreter. The methods of the interface * may be implemented using the codeinvokeFunction/code method. * + * @param T the type of the interface to return * @param clasz The codeClass/code object of the interface to return. * * @return An instance of requested interface - null if the requested interface is unavailable, @@ -95,6 +97,7 @@ * a scripting object compiled in the interpreter. The methods of the * interface may be implemented using the codeinvokeMethod/code method. * + * @param T the type of the interface to return * @param thiz The scripting object whose member functions are used to implement the methods of the interface. * @param clasz The codeClass/code object of the interface to return. * diff -r b8f16cb2d95b src/share/classes/javax/script/ScriptContext.java --- a/src/share/classes/javax/script/ScriptContext.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/ScriptContext.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -78,6 +78,7 @@ * @return The associated codeBindings/code. Returns codenull/code if it has not * been set. * + * @param scope The scope * @throws IllegalArgumentException If no codeBindings/code is defined for the * specified scope value in codeScriptContext/code of this type. */ diff -r b8f16cb2d95b src/share/classes/javax/script/ScriptEngineFactory.java --- a/src/share/classes/javax/script/ScriptEngineFactory.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/ScriptEngineFactory.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -80,6 +80,7 @@ * identify the codeScriptEngine/code by the codeScriptEngineManager/code. * For instance, an implementation based on the Mozilla Rhino Javascript engine might * return list containing {quot;javascriptquot;, quot;rhinoquot;}. + * @return an immutable list of short names */ public ListString getNames(); diff -r b8f16cb2d95b src/share/classes/javax/script/SimpleScriptContext.java --- a/src/share/classes/javax/script/SimpleScriptContext.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/SimpleScriptContext.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -82,7 +82,9 @@ */ protected Bindings globalScope; - +/** + * Create a {@code SimpleScriptContext}. + */ public SimpleScriptContext() { engineScope = new SimpleBindings(); globalScope = null;
Re: JDK 8 core review request for doclint warnings in javax.script
looks fine joe On Jun 27, 2013, at 3:47 PM, Joe Darcy wrote: Hello, Please review the next patch in a series of patches to resolve doclint warnings in the JDK; this time in javax.script. Thanks, -Joe diff -r b8f16cb2d95b src/share/classes/javax/script/Invocable.java --- a/src/share/classes/javax/script/Invocable.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/Invocable.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -63,6 +63,7 @@ /** * Used to call top-level procedures and functions defined in scripts. * + * @param name of the procedure or function to call * @param args Arguments to pass to the procedure or function * @return The value returned by the procedure or function * @@ -79,6 +80,7 @@ * the interpreter. The methods of the interface * may be implemented using the codeinvokeFunction/code method. * + * @param T the type of the interface to return * @param clasz The codeClass/code object of the interface to return. * * @return An instance of requested interface - null if the requested interface is unavailable, @@ -95,6 +97,7 @@ * a scripting object compiled in the interpreter. The methods of the * interface may be implemented using the codeinvokeMethod/code method. * + * @param T the type of the interface to return * @param thiz The scripting object whose member functions are used to implement the methods of the interface. * @param clasz The codeClass/code object of the interface to return. * diff -r b8f16cb2d95b src/share/classes/javax/script/ScriptContext.java --- a/src/share/classes/javax/script/ScriptContext.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/ScriptContext.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -78,6 +78,7 @@ * @return The associated codeBindings/code. Returns codenull/code if it has not * been set. * + * @param scope The scope * @throws IllegalArgumentException If no codeBindings/code is defined for the * specified scope value in codeScriptContext/code of this type. */ diff -r b8f16cb2d95b src/share/classes/javax/script/ScriptEngineFactory.java --- a/src/share/classes/javax/script/ScriptEngineFactory.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/ScriptEngineFactory.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -80,6 +80,7 @@ * identify the codeScriptEngine/code by the codeScriptEngineManager/code. * For instance, an implementation based on the Mozilla Rhino Javascript engine might * return list containing {quot;javascriptquot;, quot;rhinoquot;}. + * @return an immutable list of short names */ public ListString getNames(); diff -r b8f16cb2d95b src/share/classes/javax/script/SimpleScriptContext.java --- a/src/share/classes/javax/script/SimpleScriptContext.javaThu Jun 27 12:24:26 2013 -0700 +++ b/src/share/classes/javax/script/SimpleScriptContext.javaThu Jun 27 12:45:29 2013 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -82,7 +82,9 @@ */ protected Bindings globalScope; - +/** + * Create a {@code SimpleScriptContext}. + */ public SimpleScriptContext() { engineScope = new SimpleBindings(); globalScope = null; Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
hg: jdk8/tl/jdk: 8019224: add exception chaining to RMI CGIHandler
Changeset: 6729f7ef94cd Author:smarks Date: 2013-06-27 13:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6729f7ef94cd 8019224: add exception chaining to RMI CGIHandler Reviewed-by: darcy ! src/share/classes/sun/rmi/transport/proxy/CGIHandler.java
Re: hg: jdk8/tl/langtools: 8019308: Add descriptions of Java SE 7 and 8 language changes to SourceVersion
On 06/27/2013 08:49 PM, joe.da...@oracle.com wrote: Changeset: 065f8cb7bd89 Author:darcy Date: 2013-06-27 11:46 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/065f8cb7bd89 8019308: Add descriptions of Java SE 7 and 8 language changes to SourceVersion Reviewed-by: jjg ! src/share/classes/javax/lang/model/SourceVersion.java too late for this changeset, but I think that enhanced inference is also a feature of 8. Rémi
hg: jdk8/tl/jdk: 8019320: Fix doclint issues in javax.script
Changeset: 1099fe14fb65 Author:darcy Date: 2013-06-27 14:11 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/1099fe14fb65 8019320: Fix doclint issues in javax.script Reviewed-by: lancea ! src/share/classes/javax/script/Invocable.java ! src/share/classes/javax/script/ScriptContext.java ! src/share/classes/javax/script/ScriptEngineFactory.java ! src/share/classes/javax/script/SimpleScriptContext.java
Re: Question on HashMap change in 8011200
On 06/27/2013 10:02 AM, Shi Jun Zhang wrote: Hi, There are some isEmpty() check added into get/remove methods since 8011200 to return directly if HashMap is empty. However isEmpty is a non-final public method which can be overridden by subclass. If the subclass defines isEmpty differently from HashMap, it would cause problem while getting or removing elements. yes, it's a bug. Could you report it ? Rémi For example, here is a simple test case, the subclass of HashMap always has at least one key value pair so isEmpty will never be false. /// import java.util.HashMap; public class HashMapTest { public static class NotEmptyHashMapK,V extends HashMapK,V { private K alwaysExistingKey; private V alwaysExistingValue; @Override public V get(Object key) { if (key == alwaysExistingKey) { return alwaysExistingValue; } return super.get(key); } @Override public int size() { return super.size() + 1; } @Override public boolean isEmpty() { return size() == 0; } } public static void main(String[] args) { NotEmptyHashMapString, String map = new NotEmptyHashMap(); map.get(key); } } // The test can end successfully before 8011200 but it will throw ArrayIndexOutOfBoundsException after 8011200. The reason is isEmpty() check in HashMap.getEntry() method gets passed but the actual table length is 0. I think the real intention of isEmpty() check is to check whether size in HashMap is 0 but not whether the subclass is empty, so I suggest to use size == 0 instead of isEmpty() in get/remove methods. Do you think this is a bug or a wrong usage of extending HashMap? I find there is a similar usage in javax.swing.MultiUIDefaults which extends Hashtable.
Re: Review request for JDK-8016760: failure of regression test langtools/tools/javac/T6725036.java
Ok. I will put it through shortly. On 06/27/13 14:18, Jonathan Gibbons wrote: Yes, Per the latest conventions, the line should be of the form @ignore BUGID: synopsis where BUGID is the number for a currently open issue that justifies this test being ignored. -- Jon On 06/27/2013 10:31 AM, Eric McCorkle wrote: I could easily replace the patch I have now with one that just marks the test @ignore and commit it. Do we want to go ahead and approve that? On 06/27/13 13:12, Jonathan Gibbons wrote: Yes, I think that this is the correct approach. -- Jon On 06/27/2013 09:43 AM, Kumar Srinivasan wrote: If this is to be undone after the correct zip fix, why not add the @ignore for now ? and enable this when 8015666 is fixed correctly. Kumar This is fine to be a workaround for the test case for now. It probably will need to be undo-ed after the propose change for #8015666 get integrated. http://cr.openjdk.java.net/~sherman/8015666/webrev/ The proposal for #8015666 is to keep the existing behavior of ZipEntry.getTime() to return a LastModifiedTime converted from the zip entry's ms-dos-formatted date/time field by using the default timezone. A new pair ZipEntry.get/setLastModifiedTime() will be added to access the real UTC time stored in the zip entry, if presents. The API doc will be updated accordingly as well to explicitly explain the source of the date/time and the its timezone sensitive conversion. -Sherman On 06/25/2013 07:03 AM, Eric McCorkle wrote: Hello, Please review this simple patch which updates regression test langtools/tools/javac/T6725036.java to offset the time returned by JavaFileObject.getLastModified() with the local time to UTC delta. Please note that this patch is intended to address the test failures, and that I will be immediately opening a new bug to investigate and address deeper issues, and also to properly document the API. The webrev is here: http://cr.openjdk.java.net/~emc/8016760/ The bug report is here: http://bugs.sun.com/view_bug.do?bug_id=8016760 Thanks, Eric
hg: jdk8/tl/jdk: 6609431: (rb) ResourceBundle.getString() returns incorrect value
Changeset: e34e3ddb3cd8 Author:naoto Date: 2013-06-27 14:40 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e34e3ddb3cd8 6609431: (rb) ResourceBundle.getString() returns incorrect value Reviewed-by: okutsu, sherman ! src/share/classes/java/util/Properties.java + test/java/util/Properties/Bug6609431.java + test/java/util/Properties/Bug6609431.properties
hg: jdk8/tl/langtools: 8013357: javac accepts erroneous binary comparison operations
Changeset: 5c548a8542b8 Author:emc Date: 2013-06-27 17:45 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/5c548a8542b8 8013357: javac accepts erroneous binary comparison operations Summary: javac does not report type errors on illegal Object == primitive comparisons Reviewed-by: abuckley, mcimadamore ! src/share/classes/com/sun/tools/javac/code/Types.java ! src/share/classes/com/sun/tools/javac/comp/Attr.java ! test/tools/javac/lambda/LambdaConv01.java ! test/tools/javac/lambda/LambdaExpr15.java ! test/tools/javac/lambda/typeInference/InferenceTest2b.java + test/tools/javac/types/TestComparisons.java
JDK 8 code review request for 8019357: Fix doclint warnings in java.lang.invoke
Hi John, Please review the changes for 8019357: Fix doclint warnings in java.lang.invoke http://cr.openjdk.java.net/~darcy/8019357.0/ Thanks, -Joe
Re: JDK 8 code review request for 8019357: Fix doclint warnings in java.lang.invoke
On Jun 27, 2013, at 6:43 PM, Joe Darcy joe.da...@oracle.com wrote: Hi John, Please review the changes for 8019357: Fix doclint warnings in java.lang.invoke http://cr.openjdk.java.net/~darcy/8019357.0/ I read it and it is good. Thanks for doing this. — John
hg: jdk8/tl/jdk: 8019357: Fix doclint warnings in java.lang.invoke
Changeset: 29136bc5 Author:darcy Date: 2013-06-27 19:02 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/29136bc5 8019357: Fix doclint warnings in java.lang.invoke Reviewed-by: jrose ! src/share/classes/java/lang/invoke/LambdaConversionException.java ! src/share/classes/java/lang/invoke/LambdaMetafactory.java ! src/share/classes/java/lang/invoke/MethodHandle.java ! src/share/classes/java/lang/invoke/MethodHandleProxies.java ! src/share/classes/java/lang/invoke/MethodHandles.java ! src/share/classes/java/lang/invoke/MethodType.java ! src/share/classes/java/lang/invoke/MutableCallSite.java ! src/share/classes/java/lang/invoke/SerializedLambda.java ! src/share/classes/java/lang/invoke/package-info.java
hg: jdk8/tl/jdk: 8019359: To comment why not use no_renegotiation to reject client initiated renegotiation
Changeset: 60d1994f63f7 Author:xuelei Date: 2013-06-27 19:22 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/60d1994f63f7 8019359: To comment why not use no_renegotiation to reject client initiated renegotiation Reviewed-by: wetmore ! src/share/classes/sun/security/ssl/ServerHandshaker.java