On Fri, 28 Mar 2025 15:19:46 GMT, Rohitash <[email protected]> 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
test/jdk/java/net/URI/Test.java line 1809:
> 1807:
> testCreate("https://123.example.com";).s("https").h("123.example.com").p("").z();
> 1808:
> testCreate("https://1234.example.com";).s("https").h("1234.example.com").p("").z();
> 1809:
> testCreate("https://12345.example.com";).s("https").h("12345.example.com").p("").z();
can we add some URIs similar to the offending URI here? Something like:
https://98765432101.www.example.com/ and
"https://9223372036854775808.www.example.com/";
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24295#discussion_r2042014142
