Thank you all for the feedback. I have included feedback and updated
webrev.
Before :
Benchmark Mode Cnt Score Error Units
ThreadLocalURI.uriDecoderBaseline avgt 25 556.315 ± 23.389 ns/op
ThreadLocalURI.uriDecoderTest avgt 25 728.413 ± 19.833 ns/op
ThreadLocalURI.uriEncoderTest avgt 25 337.042 ± 8.304 ns/op
After :
Benchmark Mode Cnt Score Error Units
ThreadLocalURI.uriDecoderBaseline avgt 25 578.108 ± 9.829 ns/op
ThreadLocalURI.uriDecoderTest avgt 25 688.298 ± 10.654 ns/op
ThreadLocalURI.uriEncoderTest avgt 25 336.660 ± 2.997 ns/op
webrev:
https://cr.openjdk.java.net/~ryadav/webrev_8245308/webrev.01/index.html
- rahul
On 27/08/2020 15:58, Daniel Fuchs wrote:
Hi Rahul,
The code changes look good to me but the uriDecoderTest needs
some working. The reason is that the decoded path is cached
in the URI, so only the first call to getPath() will decode it.
The following calls will just return the cached value.
I also believe you should remove the inner loop - it probably serves
no purpose, and possibly find a way to pass a fresh URI to each
invocation of the uriDecoderTest method. I am not 100% sure on
how to achieve that with JMH though, but hopefully there's some
way to do that.
best regards,
-- daniel
On 27/08/2020 14:11, Rahul Yadav wrote:
Hello,
Request to have my fix reviewed for issue:
JDK-8245308: Replace ThreadLocalCoders decoder/encoder cache in
java.net.URI.
This fix updates java.net.URI, replaces the ThreadLocalCoders
optimization.
The benchmark results indicate no impact, optimization can be
replaced.The results of
benchmark are provided below and benchmark included in webrev.
Before
Benchmark (iterations) Mode Cnt Score Error
Units
ThreadLocalURI.uriDecoderTest 100 avgt 25 0.258 ±
0.014 us/op
ThreadLocalURI.uriEncoderTest 100 avgt 25 35.540 ±
1.082 us/op
After
Benchmark (iterations) Mode Cnt Score Error
Units
ThreadLocalURI.uriDecoderTest 100 avgt 25 0.247 ±
0.005 us/op
ThreadLocalURI.uriEncoderTest 100 avgt 25 37.231 ±
0.669 us/op
Issue: https://bugs.openjdk.java.net/browse/JDK-8245308
webrev: https://cr.openjdk.java.net/~ryadav/webrev_8245308/index.html
- rahul
