Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Maurizio Cimadamore
Looks great - the only minor quibble is that now StringConcat looks like a regular javac context class; it even has an instance method - it's therefore best to follow usual initialization pattern for javac components: i.e. protected static final Context.Key concatKey = new

Re: RFR [9] 8141615: Add new public methods to sun.reflect.ConstantPool

2015-11-27 Thread Konstantin Shefov
Coleen, Thanks for review On 11/24/2015 07:33 PM, Coleen Phillimore wrote: I have a couple preliminary comments. First, there are no tests added with all this new functionality. Tests should be added with the functionality changeset, not promised afterward. I will add tests. Secondly, I

Re: RFR: JDK-8142936:Additional java.time.Duration methods

2015-11-27 Thread Stephen Colebourne
"Overall, looks pretty good. There a a number of double spaces that should be single spaces in the Javadoc. Change "This is based on the standard definition of a day has 24 hours." to "This is based on the standard definition of a day as 24 hours." ("has" to "as") There are three places to fix

Re: RFR: 8140344: add support for 3 digit update release numbers

2015-11-27 Thread Seán Coffey
Looks fine to me Rob. Assuming JPRT job builds & tests ok. Regards, Sean. On 24/11/15 12:40, Rob McKenna wrote: You would think, but this fix isn't in jdk_util.c in 6. The test isn't in 6 either though. -Rob On 24/11/15 04:39, David Holmes wrote: On 24/11/2015 12:24 AM, Rob McKenna

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-27 Thread Alan Bateman
On 18/11/2015 21:27, Steve Drach wrote: Hi, Please review the latest iteration of the webrev for runtime support of multi-release jar files. I think the latest looks okay and addressed all the issues that I was concerned with. -Alan

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Aleksey Shipilev
Hi Andrej, On 11/27/2015 03:29 PM, Andrej Golovnin wrote: >> http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.03/ > > src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java > > The inner class Key is final. But the inner classes Recipe and > RecipeElement are not final.

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Aleksey Shipilev
Hi Maurizio, Updated webrevs: http://cr.openjdk.java.net/~shade/8085796/webrev.langtools.03/ http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.04/ On 11/27/2015 04:03 AM, Maurizio Cimadamore wrote: > Thanks! The patch looks better, and having the code all in one place > definitively helps.

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Aleksey Shipilev
Hi, Again, I'll post the updates after addressing another round of Maurizio's comments :) On 11/27/2015 10:47 AM, Andrej Golovnin wrote: >> http://cr.openjdk.java.net/~shade/8085796/webrev.langtools.02/ > > test/tools/javac/T5024091/T5024091.java >

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Ivan Gerasimov
Hi Alexey! Not a thorough review, but a few of minor comments: 1) As you touch Long.java anyway, could you change 508 while (i <= Integer.MIN_VALUE) { to 508 while (i < Integer.MIN_VALUE) { , which may save us a half of nanosecond on some inputs? And the same on the line# 563.

RFC: AbstractStringBuilder sharing its buffer with String

2015-11-27 Thread Ivan Gerasimov
Hello! Prior to Java5, StringBuffer used to be able to share its internal char[] buffer with the String, returned with toString(). With introducing of the new StringBuilder class this functionality was removed. It seems tempting to reintroduce this feature now in AbstractStringBuilder. The

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Andrej Golovnin
Hi Aleksey, > http://cr.openjdk.java.net/~shade/8085796/webrev.jdk.03/ src/java.base/share/classes/java/lang/invoke/StringConcatFactory.java The inner class Key is final. But the inner classes Recipe and RecipeElement are not final. Why? The #equals method of the Recipe class has this line:

Re: Code (Pre-)Review for JEP 280: Indify String Concat

2015-11-27 Thread Aleksey Shipilev
Hi Ivan, Thanks for the reviews. Again, I will publish the updated webrevs after rewiring for Maurizio's comments. On 11/27/2015 02:33 PM, Ivan Gerasimov wrote: > 1) As you touch Long.java anyway, could you change > 508 while (i <= Integer.MIN_VALUE) { > to > 508 while (i <

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

2015-11-27 Thread Magnus Ihse Bursie
On 2015-11-25 02:54, Iris Clark wrote: Hi. Please review the new classes jdk.Version and jdk.OracleVersion. These are simple Java APIs to parse, validate, and compare version numbers. Bug 8072379: Implement jdk.Version and jdk.OracleVersion

Re: RFC: AbstractStringBuilder sharing its buffer with String

2015-11-27 Thread Peter Levart
On 11/27/2015 01:39 PM, Ivan Gerasimov wrote: Hello! Prior to Java5, StringBuffer used to be able to share its internal char[] buffer with the String, returned with toString(). With introducing of the new StringBuilder class this functionality was removed. It seems tempting to reintroduce

Re: Reference.reachabilityFence

2015-11-27 Thread Paul Sandoz
Hi Peter, > On 27 Nov 2015, at 08:36, Peter Levart wrote: > > Hi Paul, > > Just a note on the test logic... > > 71 boolean finalized = false; > 72 for (int c = 0; c < MAIN_ITERS; c++) { > 73 finalized |= nonFenced(LOOP_ITERS); > 74

Re: RFR 8143628: Fork sun.misc.Unsafe and jdk.internal.misc.Unsafe native method tables

2015-11-27 Thread Paul Sandoz
> On 27 Nov 2015, at 01:59, David Holmes wrote: > > Hi Paul, > > On 26/11/2015 7:55 PM, Paul Sandoz wrote: >> Hi, >> >> This is a request for an optimistic review to fork the sun.misc.Unsafe and >> jdk.internal.misc.Unsafe native method tables so that we can evolve