RFR [9] 8006627: UUID to/from String performance should be improved by reducing object allocations (original) (raw)

Claes Redestad claes.redestad at oracle.com
Mon Jul 21 19:21:42 UTC 2014


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



More information about the core-libs-dev mailing list