On 2014-08-13 21:49, Xueming Shen wrote:

looks fine. Thanks for reviewing!

though arguably it might be more correct to do

if (dayStart != -1 || hourStart != -1 || minuteStart != -1 || secondStart != -1) { ... }

as the regex spec says the start/end return -1 if no match for the group.

start(int)[1] will return either the start index of the group (0 <= index < text.length(), implying index >= 0) or -1, so I could argue both ways are fine, no? Thanks! /Claes [1] http://docs.oracle.com/javase/8/docs/api/java/util/regex/Matcher.html#start-...

-Sherman

On 08/13/2014 05:50 AM, Claes Redestad wrote:

...

Hi,

can I have a review of this performance improvement to java.time.Period#parse and java.time.Duration#parse?

bug: https://bugs.openjdk.java.net/browse/JDK-8055004 webrev: http://cr.openjdk.java.net/~redestad/8055004/webrev.0

The patch avoids String allocation through matcher.group(n) calls, utilizes recently added capability of parsing CharSequence offsets and improves on the rather awkward way fractions of a second was aligned to nanosecond precision in java.time.Duration.

Microbenchmarks show a 1.2-1.5x throughput improvement in both cases.

Thanks!

/Claes