On Fri, 28 Mar 2025 15:19:46 GMT, Rohitash <d...@openjdk.org> wrote: > `scanByte` throws `NumberFormatException` for URIs that start with numbers, > e.g., https://11111111.x.y/ > The current flow is `parseIPv4Address` → `scanIPv4Address` → `scanByte`. > `parseIPv4Address` uses `NumberFormatException` for control flow, so it > captures the exception, ignores it, and returns -1. This has been reported by > AWS customer to cause low performance. Details: > [JDK-8353013](https://bugs.openjdk.org/browse/JDK-8353013) & > https://github.com/aws/aws-sdk-java-v2/issues/5933 > > This PR avoids NumberFormatException by skipping calls to `Integer.parseInt` > if the number of digits in the octet is > 3. > > I benchmarked on local machine for potential regressions. > https://gist.github.com/rk-kmr/cb1a3d59225c17b180a29cc125ebf887 > > > Benchmark Mode Cnt Score > Error Units > URIBenchMark.newImplWithNormalUrl thrpt 10 0.004 ± > 0.001 ops/ns > URIBenchMark.newImplWithNumberlUrl thrpt 10 0.004 ± > 0.001 ops/ns > URIBenchMark.oldImplWithNormalUrl thrpt 10 0.004 ± > 0.001 ops/ns > URIBenchMark.oldImplWithNumUrl thrpt 10 0.001 ± > 0.001 ops/ns > URIBenchMark.newImplWithNormalUrl avgt 10 236.762 ± > 8.700 ns/op > URIBenchMark.newImplWithNumberlUrl avgt 10 264.017 ± > 7.274 ns/op > URIBenchMark.oldImplWithNormalUrl avgt 10 233.853 ± > 6.539 ns/op > URIBenchMark.oldImplWithNumUrl avgt 10 1183.572 ± > 29.242 ns/op > > > I ran following tests. > > make test-tier1 > make test-tier2 > make test TEST=jdk/java/net
Could you please also add a test for this change/ add a bug id to the existing test if there already is one covering this functionality. Just in case src/java.base/share/classes/java/net/URI.java line 3429: > 3427: int q = scan(p, n, L_DIGIT, H_DIGIT); > 3428: if (q <= p) return q; > 3429: Nit: Copyright year src/java.base/share/classes/java/net/URI.java line 3435: > 3433: > 3434: // Calculate the number of significant digits (after > leading zeros) > 3435: int significantDigits = q - i; Nit: Do you think it might be more clear if this is named something like `significantDigitsNum` or similar? No strong feelings, just confused me a little when I was reading this. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24295#issuecomment-2766357418 PR Review Comment: https://git.openjdk.org/jdk/pull/24295#discussion_r2020971319 PR Review Comment: https://git.openjdk.org/jdk/pull/24295#discussion_r2020969734
