Re: Fwd: JDK 9 RFR of JDK-8030942: Explicitly state floating-point summation requirements on non-finite inputs
On 07/18/2014 12:00 PM, Georgiy Rakov wrote: On 18.07.2014 20:14, Joe Darcy wrote: Hello Georgiy, On 07/18/2014 05:29 AM, Georgiy Rakov wrote: Hello Joe, could you please clarify by short example following assertion: 154 * If the exact sum is infinite, a properly-signed infinity is 155 * returned. I'm afraid I don't quite understand what you mean here by 'exact sum'. By "exact sum," the sum absent any floating-point rounding, the sum you would get using infinite precision to operate on the values in question. The sentence in question is intended to be a short way of saying "If you have same-signed infinities in your input, the result will be an infinity of that sign." In particular, this disallows the behavior that was fixed before JDK 8 GA where having infinities in the input would cause a NaN to be returned because of how the compensated summation code manipulated those values. Thanks, I see, however it seems to me a bit confusing, since the term "infinite exact sum" seems to me not obvious and I believe it needs some definition. I'd like to suggest to use more straightforward approach, that is as you've said: "If you have same-signed infinities in your input, the result will be an infinity of that sign.". I believe it would be more clear for end user (at least for me :)) and from conformance point of view. Besides it seems to me a bit questionable. For instance "inexact some" looks like more appropriate, since overflowing to infinity occurs when _actual _sum exceeds the limit. By actual sum I mean sum resulted from actual summation with all the rounding happened. There wouldn't be such questions, provided straightforward approach is used. Thanks, Georgiy. In response to previous feedback, I propose this revised change to the specification: --- a/src/share/classes/java/util/DoubleSummaryStatistics.java Sat Jul 19 11:22:08 2014 +0800 +++ b/src/share/classes/java/util/DoubleSummaryStatistics.java Mon Jul 21 18:02:54 2014 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2014, 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 @@ -129,9 +129,6 @@ * Returns the sum of values recorded, or zero if no values have been * recorded. * - * If any recorded value is a NaN or the sum is at any point a NaN - * then the sum will be NaN. - * * The value of a floating-point sum is a function both of the * input values as well as the order of addition operations. The * order of addition operations of this method is intentionally @@ -143,6 +140,44 @@ * numerical sum compared to a simple summation of {@code double} * values. * + * Because of the unspecified order of operations and the + * possibility of using differing summation schemes, the output of + * this method may vary on the same input values. + * + * Various conditions can result in a non-finite sum being + * computed. This can occur even if the all the recorded values + * being summed are finite. If any recorded value is non-finite, + * the sum will be non-finite: + * + * + * + * If any recorded value is a NaN, then the final sum will be + * NaN. + * + * If the recorded values contain one or more infinities, the + * sum will be infinite or NaN. + * + * + * + * If the recorded values contain infinities of opposite sign, + * the sum will be NaN. + * + * If the recorded values contain infinities of one sign and + * an intermediate sum overflows to an infinity of the opposite + * sign, the sum may be NaN. + * + * + * + * + * + * It is possible for intermediate sums of finite values to + * overflow into opposite-signed infinities; if that occurs, the + * final sum will be NaN even if the recorded values are all + * finite. + * + * If all the recorded values are zero, the sign of zero is + * not guaranteed to be preserved in the final sum. + * * @apiNote Values sorted by increasing absolute magnitude tend to yield * more accurate results. * @@ -193,15 +228,9 @@ * Returns the arithmetic mean of values recorded, or zero if no * values have been recorded. * - * If any recorded value is a NaN or the sum is at any point a NaN - * then the average will be code NaN. - * - * The average returned can vary depending upon the order in - * which values are recorded. - * - * This method may be implemented using compensated summation or - * other technique to reduce the error bound in the {@link #getSum - * numerical sum} used to compute the average. + * The computed average can vary numerically and have th
Re: RFR [8051382] Optimize java.lang.reflect.Modifier.toString()
On Mon, Jul 21, 2014 at 10:58 AM, Joe Darcy wrote: > Hello, > > As a general comment, I think we should use more of StringJoiner in the > JDK libraries; it would help get rid of some awkward loops, even if it > isn't that compelling a code benefit in this case. > > In performance critical contexts, one can almost always do a little better than to use StringJoiner. StringJoiner feels like an odd API. For performance critical uses, there is no way to presize the output. As a community, we have still not learned that use of constructors (instead of factory methods) is usually a design mistake. > While performance is an important concern, I don't know if producing > modifier strings is actually performance critical. > > I agree that Modifier.toString is probably not performance critical.
Re: Map.compute() confusing Javadoc
Hi Roman; Somewhat unfortunately, "just return null" is what the default and all conforming implementations do of compute do when presented with a Map containing a mapping to null and a mapping function returning null. The specification of the new Java 8 Map methods does not always handle maps containing null values as clean as might be wished. This is mostly to maintain consistent semantics with some pre-java 8 ConcurrentHashMap operations. The chain of debatable decisions all starts with "putIfAbsent()" and then tries to proceed consistently to the other new default operations. In general a null-value mapping in a Map is treated as absent--the same way as CHM would treat a get() returning null. Every attempt I made at improving the behaviour of mapping to null values ended up being weirder and more mysterious than what Java 8 shipped with. Sorry. Mike On Jul 20 2014, at 15:25 , David Holmes wrote: > See: > > http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018251.html > > and related bug report. > > David > > On 21/07/2014 6:01 AM, Paul Sandoz wrote: >> Begin forwarded message: >>> Пересылаемое сообщение >>> 17.07.2014, 19:20, "Roman Leventov" : >>> >>> Map.compute() Javadoc has the following paragraph: >>> >>> The default implementation is equivalent to performing the following steps >>> for this map, then returning the current value or null if absent: >>> >>> V oldValue = map.get(key); >>> V newValue = remappingFunction.apply(key, oldValue); >>> if (oldValue != null ) { >>> if (newValue != null) >>>map.put(key, newValue); >>> else >>>map.remove(key); >>> } else { >>> if (newValue != null) >>>map.put(key, newValue); >>> else >>>return null; >>> } >>> >>> >>> But this code don't correspond neither verbal description of the behaviour >>> nor the actual default implementation in java.util.Map. If the oldValue is >>> null and newValue is null, map should still remove the key, not to just >>> `return null;` >>> >>
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
On 2014-07-21 22:05, Peter Levart wrote: On 07/21/2014 09:21 PM, Claes Redestad wrote: Hi, was asked offline to add a length check at the start (prevents potentially costly scans for huge, invalid input; negligible performance impact for normal cases) and realized dash1 < 0 || dash2 < 0 || dash3 < 0 implies dash4 < 0, so the first three checks are unnecessary and can be skipped: http://cr.openjdk.java.net/~redestad/8006627/webrev.5 Hi Claes, dash1 < 0 || dash2 < 0 || dash3 < 0 does not imply dash4 < 0 Take for example the following input: "0-0-0" dash1 = 1 dash2 = 3 dash3 = -1 dash4 = 1 (again) Well, this is embarrassing... but the following check: if (dash4 < 0 || name.indexOf('-', dash4 + 1) > 0) is true in this case. It seems either dash4 < 0 or dash5 > 0 for any number of dashes in the string except exactly 4. It's just that this is not immediately evident for the casual reader. I recommend adding a comment for posterity. ... but that explains why all test cases worked and reinforced the erroneous assumption. Skipping the checks bring back a few percent in my microbenchmarks, so I guess adding an explanation as to why this actually works in context in an inline comment is the least we should do here: +// For any valid input, dash1 through dash4 will be positive and dash5 +// negative, but it's enough to check dash4 and dash5: +// - if dash1 is -1, dash4 will be -1 +// - if dash1 is positive but dash2 is -1, dash4 will be -1 +// - if dash1 and dash2 is positive, dash3 will be -1, dash4 will be +// positive, but so will dash5 http://cr.openjdk.java.net/~redestad/8006627/webrev.6 Thanks! /Claes Regards, Peter /Claes On 2014-07-21 20:32, Claes Redestad wrote: Hi, new webrev which ensures we always throw some kind of IAE for invalid inputs and adds a few tests to cover this behavior: http://cr.openjdk.java.net/~redestad/8006627/webrev.4 /Claes On 2014-07-21 20:05, Claes Redestad wrote: Hi, IIOB is invalid to throw here, so I'll fix that. NumberFormatException is a IllegalArgumentException, so I think it's a gray area if the dash4 + 1 < name.length()-check is needed. I sincerely hope keeping error messages as-is isn't required, though. /Claes On 2014-07-21 18:51, Peter Levart wrote: Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: please review draft JEP: Convenience Factory Methods for Collections
On 7/19/14 2:04 AM, Remi Forax wrote: You can combine these 2 approachesi using a Ruby like builder approach with a lambda, which provide a builder object (so static method call are replaced by instance method call) avoiding the creation of too many entry objects Map.of(b -> b .entry(k0, v0) .entry(k1, v1) ... .entry(kN, vN)) Very interesting! I'll take a look at this approach. Some of the internal feedback on this JEP draft before I posted it here was, (jokingly) "What, it doesn't use lambda??" Now we might have a way to work lambda into this. :-) s'marks
Re: please review draft JEP: Convenience Factory Methods for Collections
On 7/18/14 4:41 PM, David M. Lloyd wrote: On 07/18/2014 06:36 PM, Stuart Marks wrote: Map.of() .add(k0, v0) .add(k1, v1) ... .add(kN, vN); this would result in O(N^2) performance and space allocation, though most of the allocated space is garbage. Adding another alternative, instead of producing intermediate maps which are copied at each step for the sake of maintaining O(1) lookup, this approach could instead create a linked-list-of-pairs which would be O(n) for lookups, but also O(n) for space - you could then efficiently feed that to a "real" map constructor, especially if each "map" also implemented Map.Entry, which would make an iterator particularly lightweight, making the overall cost for, say, a HashMap be O(n). Not sure it's the "right" approach, just throwing it out there as a random Friday thought. ... now being analyzed in the cold, harsh light of a Monday morning. Er, afternoon. :-) The main difficulty here is if the type returned by add() is a Map, it has to obey all the requirements of the Map contract. That means that somebody has to enforce the uniqueness constraint of the keys. This will affect things like iterating over the map or asking for its size, and so forth. Either the extra work is done at addition time or later when certain results require respecting uniqueness. If add() returns something other than a Map, such as a temporary data structure later used to construct a Map, then pushing the items onto a linked list is quite a sensible idea. (Maybe a linked list of arrays, to reduce per-element overhead.) We don't care if the keys are unique when they're being added, since that'll be taken care of later at map construction time. But this is essentially a builder, not incremental additions to a Map. s'marks
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
On 07/21/2014 09:21 PM, Claes Redestad wrote: Hi, was asked offline to add a length check at the start (prevents potentially costly scans for huge, invalid input; negligible performance impact for normal cases) and realized dash1 < 0 || dash2 < 0 || dash3 < 0 implies dash4 < 0, so the first three checks are unnecessary and can be skipped: http://cr.openjdk.java.net/~redestad/8006627/webrev.5 Hi Claes, dash1 < 0 || dash2 < 0 || dash3 < 0 does not imply dash4 < 0 Take for example the following input: "0-0-0" dash1 = 1 dash2 = 3 dash3 = -1 dash4 = 1 (again) but the following check: if (dash4 < 0 || name.indexOf('-', dash4 + 1) > 0) is true in this case. It seems either dash4 < 0 or dash5 > 0 for any number of dashes in the string except exactly 4. It's just that this is not immediately evident for the casual reader. I recommend adding a comment for posterity. Regards, Peter /Claes On 2014-07-21 20:32, Claes Redestad wrote: Hi, new webrev which ensures we always throw some kind of IAE for invalid inputs and adds a few tests to cover this behavior: http://cr.openjdk.java.net/~redestad/8006627/webrev.4 /Claes On 2014-07-21 20:05, Claes Redestad wrote: Hi, IIOB is invalid to throw here, so I'll fix that. NumberFormatException is a IllegalArgumentException, so I think it's a gray area if the dash4 + 1 < name.length()-check is needed. I sincerely hope keeping error messages as-is isn't required, though. /Claes On 2014-07-21 18:51, Peter Levart wrote: Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
Hi, was asked offline to add a length check at the start (prevents potentially costly scans for huge, invalid input; negligible performance impact for normal cases) and realized dash1 < 0 || dash2 < 0 || dash3 < 0 implies dash4 < 0, so the first three checks are unnecessary and can be skipped: http://cr.openjdk.java.net/~redestad/8006627/webrev.5 /Claes On 2014-07-21 20:32, Claes Redestad wrote: Hi, new webrev which ensures we always throw some kind of IAE for invalid inputs and adds a few tests to cover this behavior: http://cr.openjdk.java.net/~redestad/8006627/webrev.4 /Claes On 2014-07-21 20:05, Claes Redestad wrote: Hi, IIOB is invalid to throw here, so I'll fix that. NumberFormatException is a IllegalArgumentException, so I think it's a gray area if the dash4 + 1 < name.length()-check is needed. I sincerely hope keeping error messages as-is isn't required, though. /Claes On 2014-07-21 18:51, Peter Levart wrote: Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
Hi, new webrev which ensures we always throw some kind of IAE for invalid inputs and adds a few tests to cover this behavior: http://cr.openjdk.java.net/~redestad/8006627/webrev.4 /Claes On 2014-07-21 20:05, Claes Redestad wrote: Hi, IIOB is invalid to throw here, so I'll fix that. NumberFormatException is a IllegalArgumentException, so I think it's a gray area if the dash4 + 1 < name.length()-check is needed. I sincerely hope keeping error messages as-is isn't required, though. /Claes On 2014-07-21 18:51, Peter Levart wrote: Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
Hi, IIOB is invalid to throw here, so I'll fix that. NumberFormatException is a IllegalArgumentException, so I think it's a gray area if the dash4 + 1 < name.length()-check is needed. I sincerely hope keeping error messages as-is isn't required, though. /Claes On 2014-07-21 18:51, Peter Levart wrote: Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR [8051382] Optimize java.lang.reflect.Modifier.toString()
Hello, As a general comment, I think we should use more of StringJoiner in the JDK libraries; it would help get rid of some awkward loops, even if it isn't that compelling a code benefit in this case. While performance is an important concern, I don't know if producing modifier strings is actually performance critical. Cheers, -Joe On 07/19/2014 08:58 AM, Martin Buchholz wrote: StringJoiner seems written in a style suitable for an application, not in a low-level performance-oriented style suitable for a JDK core library. But we can fix that. http://cr.openjdk.java.net/~martin/webrevs/openjdk9/StringJoiner-optimization/ On Fri, Jul 18, 2014 at 6:18 PM, Ivan Gerasimov wrote: On 19.07.2014 3:07, Martin Buchholz wrote: I took a quick look at StringJoiner. It looks to me like this won't be an optimization, because StringJoiner uses StringBuilder internally, and will actually perform more total operations. Unfortunately this is true. Microbenchmarking shows that StringJoiner makes the things 30% slower, which is sad. Then I propose another simple patch giving +15% to the speed: http://cr.openjdk.java.net/~igerasim/8051382/1/webrev/ Sincerely yours, Inan
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
It appears input like "" now triggers different exception, the spec says it should throw IAE. -Sherman On 07/21/2014 04:41 AM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
Hi Claes, Invalid inputs to UUID.fromString() behave a little different than before. IllegalArgumentException is not thrown for the following inputs: For example: "0": IllegalArgumentException: Invalid UUID string: 0 (before patch) "0": IndexOutOfBoundsException (after patch) "-0": IllegalArgumentException: Invalid UUID string: -0 (before patch) "-0": NumberFormatException (after patch) "0-0-0-0-": IllegalArgumentException: Invalid UUID string: 0-0-0-0- (before patch) "0-0-0-0-": NumberFormatException (after patch) The following input (and similar) do throw NumberFormatException as before, but messages are a little different. That's OK, I suppose. "0-0-0-x-0": NumberFormatException: For input string: "x" (before patch) "0-0-0-x-0": NumberFormatException: Error at index 1 in: "x" (after patch) "0-0-0--0": NumberFormatException: For input string: "" (before patch) "0-0-0--0": NumberFormatException: (after patch) The 1st 3 examples could be fixed by checking that dash1,2,3,4 are all > 0 and that dash4 + 1 < name.length() Regards, Peter On 07/21/2014 01:41 PM, Claes Redestad wrote: On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes
Re: RFR : JDK-8046545 launcher fix to check function return values
Hi Kumar Actually, the null check macros have different parameters. NCRV_return_value is an integer. RETURNVALUE in CHECK_JNI_RETURN is a macro, which allows us to have only the two macros: CHECK_JNI_RETURN and CHECK_JNI_RETRUN_EXCEPTION I also think it is cleaner since there are only two, and they are for JNI, to keep them self contained. Would someone be willing to review webrev-02, which contain Kumar's suggested change in the comments included with the macros. Thanks -neil On 7/19/2014 8:02 AM, Kumar Srinivasan wrote: [ Since I am sponsoring this patch, I think jcheck needs one more Reviewer besides myself] Neil, looking at your webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-02/ Can we not re-use the existing macro for null check ? #define NULL_CHECK_RETURN_VALUE(NCRV_check_pointer, NCRV_return_value) so thus your new macro would become +#define CHECK_JNI_RETURN(JNIRETURN, RETURNVALUE) \ +CHECK_JNI_RETURN_EXCEPTION(RETURNVALUE); \ -do { \ -if ((JNIRETURN) == NULL) { \ -JLI_ReportErrorMessage(JNI_ERROR); \ -RETURNVALUE; \ -} \ -} while (JNI_FALSE) +NULL_CHECK_RETURN_VALUE(JNI_RETURN, RETURN_VALUE); Kumar On 7/18/2014 10:40 AM, Neil Toda wrote: Thanks Kumar. Yes, misspoke here. Will correct the comment. On 7/18/2014 10:35 AM, Kumar Srinivasan wrote: Neil, The fix looks good. However there is an inaccuracy in the comment: + * Normally, JNI calls do not return if an exception is thrown. + * However, this behavior can change in the future, + * so check for thrown exceptions. This is not true, JNI calls *will* return if an exception is thrown, however best JNI practices dictate that a pending Exception(s) must be cleared or caught, before attempting another JNI call. Under such circumstances the return value will usually be an error or a null value. I suggest making this change to reflect this. Thanks Kumar On 7/18/2014 9:53 AM, Neil Toda wrote: Please review this launcher change. Issue: https://bugs.openjdk.java.net/browse/JDK-8046545 webrev: http://cr.openjdk.java.net/~ntoda/8046545/webrev-01/ Summary: Introduce a set of macros for launcher to be used to check for certain conditions after return from select functions.
Re: State of Serialization
On 20/07/2014 11:57, Peter Firmstone wrote: Since private methods are only be called by the ObjectOutputStream / ObjectInputStream, during de-serialisation, subclass are not responsible for calling these methods, hence subclass ProtectionDomain's are not present in the Thread's AccessControlContext and as such are missing from security checks, this is why it's currently essential for classes to ensure that de-serialisation isn't performed in a privileged context. It's more complicated than that. Even final serialisable classes may have security checks. To improve security, it would be preferable to use a deserialization constructor, required to be called by subclasses in the class hierarchies, placing their ProtectionDomains in the stack context, avoiding a number of security issues. Another benefit is the ability to use final fields, while checking invariants during construction. Certainly it would be better have a mechanism that better fitted in with non-serialisation mechanisms. Addressing this without unraveling too much when pulling on a thread, and without increasing complexity of corner cases, is non-trivial. Tom
Re: RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations
On 07/19/2014 02:59 PM, Ivan Gerasimov wrote: This looks just beautiful! Thanks! But why do you need the digits() function at all? In my opinion, using formatUnsignedLong directly would be no less clearer. Sure! http://cr.openjdk.java.net/~redestad/8006627/webrev.2/ Small improvement with client compiler; no measurable change with tiered. /Claes Sincerely yours, Ivan On 19.07.2014 8:59, Claes Redestad wrote: Hi, after recent changes, this patch has been revisited and improved slightly, primarily simplifying and speeding up the toString method slightly more: http://cr.openjdk.java.net/~redestad/8006627/webrev.1/ /Claes On 2014-06-15 00:41, Claes Redestad wrote: Hi, please review this patch to improve UUID performance, originally proposed by Steven Schlansker, rebased to use the allocation-free methods added in https://bugs.openjdk.java.net/browse/JDK-8041972 Webrev: http://cr.openjdk.java.net/~redestad/8006627/webrev.0/ Bug: https://bugs.openjdk.java.net/browse/JDK-8006627 Thanks! /Claes