Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stuart Marks
Hi Stephen, On 5/4/16 8:25 AM, Stephen Colebourne wrote: A new instance is being created each time for size-zero lists/sets/maps. This should return a singleton. Yes, this is a fairly obvious optimization. It's been on my personal to-do list, but I've moved this to a sub-task in JIRA:

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stuart Marks
On 5/4/16 6:22 AM, Alan Bateman wrote: This looks very good, but I just wonder about the iteration order varying from run to run in the Set/Map implementations. I recall the section added to the javadoc where it made it very clear that the iteration order is unspecified and subject to change and

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stuart Marks
Hi Daniel, On 5/4/16 3:08 AM, Daniel Fuchs wrote: I wonder about serial interoperability with earlier versions of the JDK. For instance it will not be possible to send a Set created with Set.of(...) in JDK 9 to a JDK 8 VM. I wonder if there is any good solution to that. You could of course use

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stuart Marks
Hi Peter, Great comments and questions. Responses below On 5/4/16 12:20 AM, Peter Levart wrote: - What's the point of 11 overloaded List.of() methods for 0 ... 10-arity if you then invoke the var-args ListN(E...input) constructor that does the array cloning with 8 of them? The same for

[9] RFR: 8152207: Perform array bound checks while getting a length of bytecode instructions

2016-05-04 Thread Artem Smotrakov
Hello, Please review two small patches for jdk and hotspot repos which add array bound checks to functions which return a length of bytecode instruction. Please see details in https://bugs.openjdk.java.net/browse/JDK-8152207 http://cr.openjdk.java.net/~asmotrak/8152207/jdk/webrev.00/

Re: RFR 8147039 : Incorrect locals and operands in compiled frames

2016-05-04 Thread Brent Christian
Thanks, Vladimir. BTW, automated build+test has revealed that my test's simple array of expected values doesn't cut it in the face of other [endian|bit]ness-es. For now, I need to skip checking of a couple of locals until something more robust can be added. Thanks, -Brent

JDK 9 RFR of 8130679: Writer/StringWriter.write methods do not specify index out bounds

2016-05-04 Thread Brian Burkhalter
Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8130679 Patch: http://cr.openjdk.java.net/~bpb/8130679/webrev.00/ Summary: Add “@throws IndexOutOfBoundsException […]” to all write() methods which accept an index parameter and for which no such throws tag is

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Remi Forax
- Mail original - > De: "Peter Levart" > À: "Stephen Colebourne" , "core-libs-dev" > > Envoyé: Mercredi 4 Mai 2016 23:58:13 > Objet: Re: RFR(m): 8139233 add initial compact immutable collection >

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Peter Levart
On 05/04/2016 05:25 PM, Stephen Colebourne wrote: I disagree with altering the iteration order. Guava's ImmutableSet and ImmutableMap have reliable iteration specified to match creation order. This aspect of the design is very useful. Stephen Are they also O(1) on lookup at the same time?

Re: Review Request JDK-8155977: Update ObjectInputStream::resolveClass and resolveProxyClass to work with platform class loader

2016-05-04 Thread Karen Kinnear
For the record, I reviewed the jvm changes and they look good. thanks, Karen > On May 4, 2016, at 3:29 PM, Mandy Chung wrote: > > The default implementation of ObjectInputStream::resolveClass and > resolveProxyClass finds the user-defined class loader on the stack and

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-04 Thread Brent Christian
The new test looks good, Mandy. Thanks, -Brent On 05/04/2016 12:36 PM, Mandy Chung wrote: This version includes a new test: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.02/ Daniel - StackFrameInfo class is package-private class In the future this may become a final class

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Xueming Shen
Thanks Claes! fixed and webrev has been updated. http://cr.openjdk.java.net/~sherman/8150496/webrev sherman On 05/04/2016 12:38 PM, Claes Redestad wrote: Hi Sherman, On 2016-05-04 21:14, Xueming Shen wrote: http://cr.openjdk.java.net/~sherman/8150496/webrev this looks good to me!

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Claes Redestad
Hi Sherman, On 2016-05-04 21:14, Xueming Shen wrote: http://cr.openjdk.java.net/~sherman/8150496/webrev this looks good to me! Pre-existing typo 'bofore' -> 'before' in src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipPath.java /Claes

Re: Review request 8153912: StackFrame::getFileName and StackFrame::getLineNumber not needed

2016-05-04 Thread Mandy Chung
This version includes a new test: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8153912/webrev.02/ Daniel - StackFrameInfo class is package-private class In the future this may become a final class once we decide what to do with locals and operands in a future release. I don’t worry about

Review Request JDK-8155977: Update ObjectInputStream::resolveClass and resolveProxyClass to work with platform class loader

2016-05-04 Thread Mandy Chung
The default implementation of ObjectInputStream::resolveClass and resolveProxyClass finds the user-defined class loader on the stack and assumes that only system classes are loaded by null loader. As JDK modules are deprivileged, classes on the stack defined by the platform class loader. These

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Aleksey Shipilev
On 05/04/2016 10:14 PM, Xueming Shen wrote: > http://cr.openjdk.java.net/~sherman/8150496/webrev Okay, good. > yes, the performance gains still hold. > > Benchmark Mode Cnt Score Error Units >

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Xueming Shen
Hi Aleksey, The webrev has been updated accordingly. http://cr.openjdk.java.net/~sherman/8150496/webrev --- *) Parentheses to nail down eval order: if (w&& zfs.isReadOnly() || x) { ...to: if ((w&& zfs.isReadOnly())

Re: RFR 8155258: VarHandle implementation improvements

2016-05-04 Thread Vladimir Ivanov
PS: I wonder whether the MH cache in VarHandle (VarHandle.typesAndInvokers.methodHandle_table) should be idempotent or not. I am guessing you mean in terms of the MH ref identity? Yes, but I think it matters only if bytecode spinning happens (it does for LambdaForms). What matters for the

Re: RFR 8155258: VarHandle implementation improvements

2016-05-04 Thread Paul Sandoz
> On 4 May 2016, at 09:19, Vladimir Ivanov wrote: > > Looks good. > > I'm fine with pushing it directly into jdk9/dev, since the change just > relaxes the assert and the risk of a merge conflict is very small. > Thanks! > Best regards, > Vladimir Ivanov > >

Re: RFR 8155258: VarHandle implementation improvements

2016-05-04 Thread Vladimir Ivanov
Looks good. I'm fine with pushing it directly into jdk9/dev, since the change just relaxes the assert and the risk of a merge conflict is very small. Best regards, Vladimir Ivanov PS: I wonder whether the MH cache in VarHandle (VarHandle.typesAndInvokers.methodHandle_table) should be

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Alexandre (Shura) Iline
This makes sense - I will move the tests into a subduer, put the dependencies into a TEST.properties file. jdk.zipfs has the code needed for access jars - the tests are failing without that dependency. Shura > On May 4, 2016, at 8:30 AM, Chris Hegarty wrote: > > On

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

2016-05-04 Thread Felix Yang
Hi Alan, On 2016/5/4 22:45, Alan Bateman wrote: On 04/05/2016 15:39, Felix Yang wrote: Hi Alan, please review the updated webrev. Reverted changes for those tests with "-addmods". http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.01/ For the javax.transaction test then don't you

Re: RFR: 8152084: Introduction of ssliop protocol to corbaloc

2016-05-04 Thread Alan Bateman
On 04/05/2016 15:53, Andrew Dinn wrote: : Just to dispel any idea that this has been plucked out of thin air by the JacORB implementors I'll note that there appears to be both a standard for and more than one implementation of ssliop. Regarding implementations, OpenORB and TAO also implemented

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Chris Hegarty
On 4 May 2016, at 14:32, Alan Bateman wrote: > > On 04/05/2016 11:24, Chris Hegarty wrote: >> : >> The tests cause compilation of test library classes, but only some tests >> actually use the methods that provoke compilation. Similar to above, tests >> that don’t

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stephen Colebourne
A new instance is being created each time for size-zero lists/sets/maps. This should return a singleton. The serialization proxy is reminiscent of those in JSR-310 but less space efficient. Once per stream, the proxy will write "java.util.ImmutableCollections.SerialProxy", whereas the JSR-310 one

Re: RFR 8155794 Move Objects.checkIndex BiFunction accepting methods to an internal package

2016-05-04 Thread Vladimir Ivanov
Looks good. Best regards, Vladimir Ivanov On 5/3/16 1:37 AM, Paul Sandoz wrote: Hi, Please review: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8155794-checkIndex-bifunc-internal-jdk/webrev/

Re: RFR: 8152084: Introduction of ssliop protocol to corbaloc

2016-05-04 Thread Andrew Dinn
On 04/05/16 15:36, Tomasz Adamski wrote: > I'm proposing this extension from practical reasons as it would > enable configuration of fully secured iiop invocations - the feature > that was present in ORB implementation that we used previously > (JacORB). ORB specification allows adding new

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Aleksey Shipilev
Hi Sherman, On 05/03/2016 11:19 PM, Xueming Shen wrote: > Again, thanks for the review. The webrev has been updated accordingly, > as well as the MyBenchmark.java (to use Blackhole, as suggested) > > http://cr.openjdk.java.net/~sherman/8150496/webrev/ Further nits, sorry for not catching them

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

2016-05-04 Thread Alan Bateman
On 04/05/2016 15:39, Felix Yang wrote: Hi Alan, please review the updated webrev. Reverted changes for those tests with "-addmods". http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.01/ For the javax.transaction test then don't you also add "java.transaction" in the @modules value?

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

2016-05-04 Thread nadeesh tv
Hi Roger, Ok. Thanks. Regards, Nadeesh On 5/4/2016 7:58 PM, Roger Riggs wrote: Hi Nadeesh, java/time/format/DateTimeFormatterBuilder.java: 1836-1839 Correct as is, but could collapse both count ==2 and count ==3 into a single appendValue call: appendValue(field, count, 3,

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

2016-05-04 Thread Felix Yang
Hi Alan, please review the updated webrev. Reverted changes for those tests with "-addmods". http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.01/ Thanks, Felix On 2016/4/29 15:25, Alan Bateman wrote: On 29/04/2016 03:16, Felix Yang wrote: Hi there, please review the changes to

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-05-04 Thread Roger Riggs
+1 Roger On 5/4/2016 6:00 AM, Stephen Colebourne wrote: Fine by me. thanks Stephen On 4 May 2016 at 08:13, nadeesh tv wrote: Hi, Updated the webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.03/ Thanks and Regards, Nadeesh On 5/3/2016 8:37 PM, Stephen

Re: RFR: 8152084: Introduction of ssliop protocol to corbaloc

2016-05-04 Thread Tomasz Adamski
I'm proposing this extension from practical reasons as it would enable configuration of fully secured iiop invocations - the feature that was present in ORB implementation that we used previously (JacORB). ORB specification allows adding new corbaloc protocols if necessary: "This

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

2016-05-04 Thread Roger Riggs
Hi Nadeesh, java/time/format/DateTimeFormatterBuilder.java: 1836-1839 Correct as is, but could collapse both count ==2 and count ==3 into a single appendValue call: appendValue(field, count, 3, SignStyle.NOT_NEGATIVE); Reviewed. Roger On 5/4/2016 3:15 AM, nadeesh tv wrote: Hi, Updated

Re: RFR 8147984: WindowsTerminal should support function keys

2016-05-04 Thread Florent Guillaume
On Wed, May 4, 2016 at 3:29 PM, Jan Lahoda wrote: > On 3.5.2016 14:58, Florent Guillaume wrote: >> http://www.x.org/docs/xterm/ctlseqs.pdf is probably a more canonical >> reference. > > It seems that this version of the document unfortunately does not specify > the codes

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Alan Bateman
On 04/05/2016 11:24, Chris Hegarty wrote: : The tests cause compilation of test library classes, but only some tests actually use the methods that provoke compilation. Similar to above, tests that don’t actually compile anything could depend on just java.compiler. This is all to fragile and

Re: RFR 8147984: WindowsTerminal should support function keys

2016-05-04 Thread Jan Lahoda
On 3.5.2016 14:58, Florent Guillaume wrote: Hi, http://www.x.org/docs/xterm/ctlseqs.pdf is probably a more canonical reference. It seems that this version of the document unfortunately does not specify the codes the terminal sends for the function keys? Jan Florent On Mon, May 2,

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Alan Bateman
On 04/05/2016 05:55, Stuart Marks wrote: Hi all, This is a reimplementation of collections created by the JEP 269 convenience factory methods. These implementations are overall quite a bit smaller than their conventional collections counterparts, particularly at small sizes. Lookup

Re: Review request: 8154190 & 8155513: Deprivilege java.compiler and jdk.charsets

2016-05-04 Thread Alan Bateman
On 03/05/2016 21:16, Mandy Chung wrote: I originally asked Sherman to identify the permissions for jdk.charsets as a follow up issue. I took another look and define the list: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8155513/webrev.01/ This looks okay to me. -Alan.

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Chris Hegarty
On 4 May 2016, at 11:07, Alan Bateman wrote: > On 04/05/2016 09:40, Chris Hegarty wrote: >> On 3 May 2016, at 16:10, Chris Hegarty wrote: >> > Can you please take a look on: > http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Alan Bateman
On 04/05/2016 09:40, Chris Hegarty wrote: On 3 May 2016, at 16:10, Chris Hegarty wrote: Can you please take a look on: http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ Taking another look over this before sponsoring…. The test library dependency seems to

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Daniel Fuchs
Hi Stuart, It will be good to have really immutable collections! But I have one comment: I wonder about serial interoperability with earlier versions of the JDK. For instance it will not be possible to send a Set created with Set.of(...) in JDK 9 to a JDK 8 VM. I wonder if there is any good

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-05-04 Thread Stephen Colebourne
Fine by me. thanks Stephen On 4 May 2016 at 08:13, nadeesh tv wrote: > Hi, > > Updated the webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.03/ > > Thanks and Regards, > Nadeesh > > On 5/3/2016 8:37 PM, Stephen Colebourne wrote: >> >> The current behaviour is to use

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-04 Thread Chris Hegarty
On 3 May 2016, at 16:10, Chris Hegarty wrote: >>> >>> Can you please take a look on: >>> http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ Taking another look over this before sponsoring…. The test library dependency seems to be on java.compiler, and not

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Remi Forax
Hi Stuart, nice work ! first, all classes should be final otherwise, they are not truly immutable and all arrays should be marked as @Stable. List0, List1, why not using Collections.emptyList(), Collections.singletonList() here ? ListN: - i don't think that extending AbstractList is a

Re: RFR 8147039 : Incorrect locals and operands in compiled frames

2016-05-04 Thread Vladimir Ivanov
Looks good. Best regards, Vladimir Ivanov On 5/3/16 10:00 PM, Brent Christian wrote: Hi, Please review this change which fixes buggy behavior (including SEGV) in the experimental LiveStackFrame feature of StackWalker. Bug: https://bugs.openjdk.java.net/browse/JDK-8147039 Webrev:

Re: RFR: JDK-8150496,(zipfs) Fix performance issues in zip-fs

2016-05-04 Thread Peter Levart
Hi Aleksey, On 05/03/2016 02:39 PM, Aleksey Shipilev wrote: *) There is ever-so-subtle difference in doing either: 171 byte[] tmp = new byte[path.length + 1]; 172 System.arraycopy(path, 0, tmp, 1, path.length); 173 tmp[0] = '/'; ...or: 1083

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Peter Levart
Hi Stuart, Good to see these implementations finally appear. I have a few comments: - What's the point of 11 overloaded List.of() methods for 0 ... 10-arity if you then invoke the var-args ListN(E...input) constructor that does the array cloning with 8 of them? The same for Set.of()... I

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

2016-05-04 Thread nadeesh tv
Hi, Updated the webev http://cr.openjdk.java.net/~ntv/8079628/webrev.04/ Regards, Nadeesh On 5/3/2016 8:45 PM, Stephen Colebourne wrote: Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m", "s" and no doubt others use NORMAL via appendValue(field); Changing these to use

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-05-04 Thread nadeesh tv
Hi, Updated the webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.03/ Thanks and Regards, Nadeesh On 5/3/2016 8:37 PM, Stephen Colebourne wrote: The current behaviour is to use NORMAL for "A" and NOT_NEGATIVE for "AA", "AAA" and so on. The sensible behaviour going forward is to use

Re: RFR 8155794 Move Objects.checkIndex BiFunction accepting methods to an internal package

2016-05-04 Thread Remi Forax
- Mail original - > De: "Paul Sandoz" > Cc: "hotspot-dev developers" , "Core-Libs-Dev" > > Envoyé: Mardi 3 Mai 2016 22:03:12 > Objet: Re: RFR 8155794 Move Objects.checkIndex BiFunction accepting