Re: RFR: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens [v5]
On Wed, 13 Jan 2021 18:59:30 GMT, Daniel Fuchs wrote: >> PING: could you review this PR? > > Hi Yasumasa, > > The new StreamCloseTest seems to be suffering from some race conditions; On > windows I saw it failing 29 times out of 50. > > config StreamCloseTest.setup(): success > test StreamCloseTest.closeTestOnException(): success > test StreamCloseTest.normallyCloseTest(): failure > java.io.IOException: HTTP/1.1 header parser received no bytes > at > java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:584) > at > java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:119) > at StreamCloseTest.normallyCloseTest(StreamCloseTest.java:134) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:566) > at > org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) > at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) > at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) > at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) > at > org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) > at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) > at org.testng.TestRunner.privateRun(TestRunner.java:773) > at org.testng.TestRunner.run(TestRunner.java:623) > at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) > at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) > at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) > at org.testng.SuiteRunner.run(SuiteRunner.java:259) > at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) > at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) > at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) > at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) > at org.testng.TestNG.run(TestNG.java:1018) > at > com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) > at > com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) > at > java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.base/java.lang.reflect.Method.invoke(Method.java:566) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:831) > Caused by: java.io.IOException: HTTP/1.1 header parser received no bytes > at > java.net.http/jdk.internal.net.http.common.Utils.wrapWithExtraDetail(Utils.java:345) > at > java.net.http/jdk.internal.net.http.Http1Response$HeadersReader.onReadError(Http1Response.java:675) > at > java.net.http/jdk.internal.net.http.Http1AsyncReceiver.checkForErrors(Http1AsyncReceiver.java:302) > at > java.net.http/jdk.internal.net.http.Http1AsyncReceiver.flush(Http1AsyncReceiver.java:268) > at > java.net.http/jdk.internal.net.http.common.SequentialScheduler$SynchronizedRestartableTask.run(SequentialScheduler.java:175) > at > java.net.http/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:147) > at > java.net.http/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:198) > at > java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1135) > at > java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) > ... 1 more > Caused by: java.io.IOException: An established connection was aborted by the > software in your host machine > at java.base/sun.nio.ch.SocketDispatcher.read0(Native Method) > at java.base/sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:46) > at java.base/sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:276) > at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:245) > at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:223) > at > java.base/sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:417) > at > java.net.http/jdk.internal.net.http.SocketTube.readAvailable(SocketTube.java:1162) > at > java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$InternalReadSubscription.read(SocketTube.java:825) > at > java.net.http/jdk.internal.net.ht
Re: RFR: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens [v5]
> `InputStream` from `BodyPublishers.ofInputStream()` is usually closed when
> the stream reaches EOF. However IOE handler returns without closing.
>
> I confirmed this problem in `BodyPublishers.ofInputStream()`, but I think
> `BodyPublishers.ofFile()`has same problem because it also use
> `StreamIterator`as well as `ofInputStream()`.
>
>
> # How to reproduce:
>
> Following code (Test.java) attempts to post body from `TestInputStream` which
> throws IOE in `read()`. "close called" is shown on the console if `close()`
> is called.
>
> import java.io.*;
> import java.net.*;
> import java.net.http.*;
>
> public class Test{
>
> private static class TestInputStream extends InputStream{
>
> public TestInputStream(){
> super();
> System.out.println("test c'tor");
> }
>
> @Override
> public int read() throws IOException{
> System.out.println("read called");
> throw new IOException("test");
> }
>
> @Override
> public void close() throws IOException{
> System.out.println("close called");
> super.close();
> }
>
> }
>
> public static void main(String[] args) throws Exception{
> var http = HttpClient.newHttpClient();
> var request = HttpRequest.newBuilder()
> .uri(URI.create("http://httpbin.org/post";))
>
> .POST(HttpRequest.BodyPublishers.ofInputStream(() -> new TestInputStream()))
> .build();
> http.send(request, HttpResponse.BodyHandlers.discarding());
> System.out.println("Press any key to exit...");
> System.in.read();
> }
> }
Yasumasa Suenaga has updated the pull request incrementally with one additional
commit since the last revision:
Use HttpServerAdapters in StreamCloseTest.java
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/1614/files
- new: https://git.openjdk.java.net/jdk/pull/1614/files/95badebc..a081290f
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1614&range=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1614&range=03-04
Stats: 45 lines in 1 file changed: 9 ins; 25 del; 11 mod
Patch: https://git.openjdk.java.net/jdk/pull/1614.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1614/head:pull/1614
PR: https://git.openjdk.java.net/jdk/pull/1614
Re: RFR: 8259662: SocketException should be passed through [v2]
On Wed, 13 Jan 2021 18:41:26 GMT, Rajan Halade wrote: >> Clive Verghese has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Fix bugids and use server port passed as a parameter > > test/jdk/sun/security/ssl/SSLContextImpl/ShouldThrowSSLExceptionWhenPeerClosesSocket.java > line 32: > >> 30: >> 31: /* >> 32: * @test > > Suggestion: > > * @test > * @bug 8259662 8259516 Thank you for the feedback. I have updated the change to include the bug ids. - PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8259662: SocketException should be passed through [v2]
> Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake Clive Verghese has updated the pull request incrementally with one additional commit since the last revision: Fix bugids and use server port passed as a parameter - Changes: - all: https://git.openjdk.java.net/jdk/pull/2057/files - new: https://git.openjdk.java.net/jdk/pull/2057/files/00667985..da98249d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2057&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2057&range=00-01 Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057 PR: https://git.openjdk.java.net/jdk/pull/2057
Re: RFR: 8257736: InputStream from BodyPublishers.ofInputStream() leaks when IOE happens [v4]
On Thu, 7 Jan 2021 00:32:42 GMT, Yasumasa Suenaga wrote: >> @ChrisHegarty @dfuch could you review new change? It updates a testcase. > > PING: could you review this PR? Hi Yasumasa, The new StreamCloseTest seems to be suffering from some race conditions; On windows I saw it failing 29 times out of 50. config StreamCloseTest.setup(): success test StreamCloseTest.closeTestOnException(): success test StreamCloseTest.normallyCloseTest(): failure java.io.IOException: HTTP/1.1 header parser received no bytes at java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:584) at java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:119) at StreamCloseTest.normallyCloseTest(StreamCloseTest.java:134) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85) at org.testng.internal.Invoker.invokeMethod(Invoker.java:639) at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821) at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131) at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:124) at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108) at org.testng.TestRunner.privateRun(TestRunner.java:773) at org.testng.TestRunner.run(TestRunner.java:623) at org.testng.SuiteRunner.runTest(SuiteRunner.java:357) at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352) at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310) at org.testng.SuiteRunner.run(SuiteRunner.java:259) at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52) at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86) at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185) at org.testng.TestNG.runSuitesLocally(TestNG.java:1110) at org.testng.TestNG.run(TestNG.java:1018) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94) at com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.io.IOException: HTTP/1.1 header parser received no bytes at java.net.http/jdk.internal.net.http.common.Utils.wrapWithExtraDetail(Utils.java:345) at java.net.http/jdk.internal.net.http.Http1Response$HeadersReader.onReadError(Http1Response.java:675) at java.net.http/jdk.internal.net.http.Http1AsyncReceiver.checkForErrors(Http1AsyncReceiver.java:302) at java.net.http/jdk.internal.net.http.Http1AsyncReceiver.flush(Http1AsyncReceiver.java:268) at java.net.http/jdk.internal.net.http.common.SequentialScheduler$SynchronizedRestartableTask.run(SequentialScheduler.java:175) at java.net.http/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:147) at java.net.http/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:198) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1135) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ... 1 more Caused by: java.io.IOException: An established connection was aborted by the software in your host machine at java.base/sun.nio.ch.SocketDispatcher.read0(Native Method) at java.base/sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:46) at java.base/sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:276) at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:245) at java.base/sun.nio.ch.IOUtil.read(IOUtil.java:223) at java.base/sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:417) at java.net.http/jdk.internal.net.http.SocketTube.readAvailable(SocketTube.java:1162) at java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$InternalReadSubscription.read(SocketTube.java:825) at java.net.http/jdk.internal.net
Re: RFR: 8259662: SocketException should be passed through
On Wed, 13 Jan 2021 06:19:18 GMT, Clive Verghese wrote: > Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake Changes requested by rhalade (Reviewer). test/jdk/sun/security/ssl/SSLContextImpl/ShouldThrowSSLExceptionWhenPeerClosesSocket.java line 32: > 30: > 31: /* > 32: * @test Suggestion: * @test * @bug 8259662 8259516 - PR: https://git.openjdk.java.net/jdk/pull/2057
Integrated: JDK-8212035 : merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer
On Fri, 4 Dec 2020 19:44:33 GMT, Mahendra Chhipa wrote: > jaxp.library.SimpleHttpServer > https://bugs.openjdk.java.net/browse/JDK-8212035 This pull request has now been integrated. Changeset: 5df2a949 Author:Mahendra Chhipa Committer: Daniel Fuchs URL: https://git.openjdk.java.net/jdk/commit/5df2a949 Stats: 651 lines in 17 files changed: 227 ins; 321 del; 103 mod 8212035: merge jdk.test.lib.util.SimpleHttpServer with jaxp.library.SimpleHttpServer Reviewed-by: dfuchs - PR: https://git.openjdk.java.net/jdk/pull/1632
Re: RFR: 8259631: Reapply pattern match instanceof use in HttpClientImpl
On Tue, 12 Jan 2021 16:05:01 GMT, Aleksei Efimov wrote: > Hi, > > The proposed change adds back [1] `instanceof` pattern match use to > `HttpClientImpl` class. It was previously removed by > [JDK-8258696](https://bugs.openjdk.java.net/browse/JDK-8258696) due to docs > build failure. > > Aleksei > > [1] `git rollback be41468c --no-commit` Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2051
Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
On Wed, 13 Jan 2021 13:48:40 GMT, Claes Redestad wrote:
>> Instead of allocating a copy of underlying array via
>> `CharArrayWriter.toCharArray()` and passing it to constructor of String
>> String str = new String(charArrayWriter.toCharArray());
>> we could call `toString()` method
>> String str = charArrayWriter.toString();
>> decoding existing char[] without making a copy. This slightly speeds up the
>> method reducing at the same time memory consumption for decoding URLs with
>> non-latin symbols:
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class UrlEncoderBenchmark {
>> private static final Charset charset = Charset.defaultCharset();
>> private static final String utf8Url =
>> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций";; // UN
>>
>> @Benchmark
>> public String encodeUtf8() {
>> return URLEncoder.encode(utf8Url, charset);
>> }
>> }
>> The benchmark on my maching give the following output:
>> before
>> BenchmarkMode Cnt
>> ScoreError Units
>> UrlEncoderBenchmark.encodeUtf8 avgt 100
>> 1166.378 ± 8.411 ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
>> 932.944 ± 6.393 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
>> 1712.193 ± 0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
>> 929.221 ± 24.268 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
>> 1705.444 ± 43.235B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
>> 0.006 ± 0.001 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
>> 0.011 ± 0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
>> 652.000 counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
>> 334.000 ms
>>
>> after
>> BenchmarkMode Cnt
>> ScoreError Units
>> UrlEncoderBenchmark.encodeUtf8 avgt 100
>> 1058.851 ± 6.006 ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
>> 931.489 ± 5.182 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
>> 1552.176 ± 0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
>> 933.491 ± 24.164 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
>> 1555.488 ± 39.204B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
>> 0.006 ± 0.001 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
>> 0.010 ± 0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
>> 655.000 counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
>> 333.000 ms
>
> Looks good.
>
> I wonder... `CharArrayWriter` is an old and synchronized data structure, and
> since the instance used here isn't shared that synchronization seem useless.
> And since you're now bypassing the `char[]` and going straight for a `String`
> you might get better performance with a `StringBuilder` here? (`setLength(0)`
> instead of `reset()`...)
@cl4es hi, let me try `StringBuilder`
-
PR: https://git.openjdk.java.net/jdk/pull/1598
Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
On Thu, 3 Dec 2020 14:29:58 GMT, Сергей Цыпанов
wrote:
> Instead of allocating a copy of underlying array via
> `CharArrayWriter.toCharArray()` and passing it to constructor of String
> String str = new String(charArrayWriter.toCharArray());
> we could call `toString()` method
> String str = charArrayWriter.toString();
> decoding existing char[] without making a copy. This slightly speeds up the
> method reducing at the same time memory consumption for decoding URLs with
> non-latin symbols:
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class UrlEncoderBenchmark {
> private static final Charset charset = Charset.defaultCharset();
> private static final String utf8Url =
> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций";; // UN
>
> @Benchmark
> public String encodeUtf8() {
> return URLEncoder.encode(utf8Url, charset);
> }
> }
> The benchmark on my maching give the following output:
> before
> BenchmarkMode Cnt
> ScoreError Units
> UrlEncoderBenchmark.encodeUtf8 avgt 100
> 1166.378 ± 8.411 ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
> 932.944 ± 6.393 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
> 1712.193 ± 0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
> 929.221 ± 24.268 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
> 1705.444 ± 43.235B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
> 0.006 ± 0.001 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
> 0.011 ± 0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
> 652.000 counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
> 334.000 ms
>
> after
> BenchmarkMode Cnt
> ScoreError Units
> UrlEncoderBenchmark.encodeUtf8 avgt 100
> 1058.851 ± 6.006 ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
> 931.489 ± 5.182 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
> 1552.176 ± 0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
> 933.491 ± 24.164 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
> 1555.488 ± 39.204B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
> 0.006 ± 0.001 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
> 0.010 ± 0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
> 655.000 counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
> 333.000 ms
Looks good.
I wonder... `CharArrayWriter` is an old and synchronized data structure, and
since the instance used here isn't shared that synchronization seem useless.
And since you're now bypassing the `char[]` and going straight for a `String`
you might get better performance with a `StringBuilder` here? (`setLength(0)`
instead of `reset()`...)
-
Marked as reviewed by redestad (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1598
Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
On Thu, 3 Dec 2020 14:29:58 GMT, Сергей Цыпанов
wrote:
> Instead of allocating a copy of underlying array via
> `CharArrayWriter.toCharArray()` and passing it to constructor of String
> String str = new String(charArrayWriter.toCharArray());
> we could call `toString()` method
> String str = charArrayWriter.toString();
> decoding existing char[] without making a copy. This slightly speeds up the
> method reducing at the same time memory consumption for decoding URLs with
> non-latin symbols:
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class UrlEncoderBenchmark {
> private static final Charset charset = Charset.defaultCharset();
> private static final String utf8Url =
> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций";; // UN
>
> @Benchmark
> public String encodeUtf8() {
> return URLEncoder.encode(utf8Url, charset);
> }
> }
> The benchmark on my maching give the following output:
> before
> BenchmarkMode Cnt
> ScoreError Units
> UrlEncoderBenchmark.encodeUtf8 avgt 100
> 1166.378 ± 8.411 ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
> 932.944 ± 6.393 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
> 1712.193 ± 0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
> 929.221 ± 24.268 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
> 1705.444 ± 43.235B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
> 0.006 ± 0.001 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
> 0.011 ± 0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
> 652.000 counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
> 334.000 ms
>
> after
> BenchmarkMode Cnt
> ScoreError Units
> UrlEncoderBenchmark.encodeUtf8 avgt 100
> 1058.851 ± 6.006 ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
> 931.489 ± 5.182 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
> 1552.176 ± 0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
> 933.491 ± 24.164 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
> 1555.488 ± 39.204B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
> 0.006 ± 0.001 MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
> 0.010 ± 0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
> 655.000 counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
> 333.000 ms
Looks good!
-
Marked as reviewed by attila (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/1598
Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
On Mon, 11 Jan 2021 09:28:46 GMT, Сергей Цыпанов wrote: >> Looks good! > > @szegedi could you please create a ticket for this change? Otherwise I cannot > merge it Filed a ticket, you should rename this PR to "8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)" - PR: https://git.openjdk.java.net/jdk/pull/1598
Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
On Sun, 10 Jan 2021 20:13:51 GMT, Attila Szegedi wrote:
>> Instead of allocating a copy of underlying array via
>> `CharArrayWriter.toCharArray()` and passing it to constructor of String
>> String str = new String(charArrayWriter.toCharArray());
>> we could call `toString()` method
>> String str = charArrayWriter.toString();
>> decoding existing char[] without making a copy. This slightly speeds up the
>> method reducing at the same time memory consumption for decoding URLs with
>> non-latin symbols:
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class UrlEncoderBenchmark {
>> private static final Charset charset = Charset.defaultCharset();
>> private static final String utf8Url =
>> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций";; // UN
>>
>> @Benchmark
>> public String encodeUtf8() {
>> return URLEncoder.encode(utf8Url, charset);
>> }
>> }
>> The benchmark on my maching give the following output:
>> before
>> BenchmarkMode Cnt
>> ScoreError Units
>> UrlEncoderBenchmark.encodeUtf8 avgt 100
>> 1166.378 ± 8.411 ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
>> 932.944 ± 6.393 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
>> 1712.193 ± 0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
>> 929.221 ± 24.268 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
>> 1705.444 ± 43.235B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
>> 0.006 ± 0.001 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
>> 0.011 ± 0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
>> 652.000 counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
>> 334.000 ms
>>
>> after
>> BenchmarkMode Cnt
>> ScoreError Units
>> UrlEncoderBenchmark.encodeUtf8 avgt 100
>> 1058.851 ± 6.006 ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
>> 931.489 ± 5.182 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
>> 1552.176 ± 0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
>> 933.491 ± 24.164 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
>> 1555.488 ± 39.204B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
>> 0.006 ± 0.001 MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
>> 0.010 ± 0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
>> 655.000 counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
>> 333.000 ms
>
> Looks good!
@szegedi could you please create a ticket for this change? Otherwise I cannot
merge it
-
PR: https://git.openjdk.java.net/jdk/pull/1598
RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)
Instead of allocating a copy of underlying array via
`CharArrayWriter.toCharArray()` and passing it to constructor of String
String str = new String(charArrayWriter.toCharArray());
we could call `toString()` method
String str = charArrayWriter.toString();
decoding existing char[] without making a copy. This slightly speeds up the
method reducing at the same time memory consumption for decoding URLs with
non-latin symbols:
@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class UrlEncoderBenchmark {
private static final Charset charset = Charset.defaultCharset();
private static final String utf8Url =
"https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций";; // UN
@Benchmark
public String encodeUtf8() {
return URLEncoder.encode(utf8Url, charset);
}
}
The benchmark on my maching give the following output:
before
BenchmarkMode Cnt
ScoreError Units
UrlEncoderBenchmark.encodeUtf8 avgt 100
1166.378 ± 8.411 ns/op
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
932.944 ± 6.393 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
1712.193 ± 0.005B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
929.221 ± 24.268 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
1705.444 ± 43.235B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
0.006 ± 0.001 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
0.011 ± 0.002B/op
UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
652.000 counts
UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
334.000 ms
after
BenchmarkMode Cnt
ScoreError Units
UrlEncoderBenchmark.encodeUtf8 avgt 100
1058.851 ± 6.006 ns/op
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt 100
931.489 ± 5.182 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm avgt 100
1552.176 ± 0.005B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space avgt 100
933.491 ± 24.164 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm avgt 100
1555.488 ± 39.204B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space avgt 100
0.006 ± 0.001 MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm avgt 100
0.010 ± 0.002B/op
UrlEncoderBenchmark.encodeUtf8:·gc.count avgt 100
655.000 counts
UrlEncoderBenchmark.encodeUtf8:·gc.time avgt 100
333.000 ms
-
Commit messages:
- Merge branch 'master' into enc
- Improve URLEncoder.encode(String, Charset)
Changes: https://git.openjdk.java.net/jdk/pull/1598/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1598&range=00
Issue: https://bugs.openjdk.java.net/browse/JDK-8259699
Stats: 8 lines in 1 file changed: 0 ins; 0 del; 8 mod
Patch: https://git.openjdk.java.net/jdk/pull/1598.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/1598/head:pull/1598
PR: https://git.openjdk.java.net/jdk/pull/1598
Re: RFR: 8259662: SocketException should be passed through
On Wed, 13 Jan 2021 06:19:18 GMT, Clive Verghese wrote: > Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears > to not be fully fixed > > This also fixes JDK-8259516: Alerts sent by peer may not be received > correctly during TLS handshake The changes to the HttpClient test look reasonable to me. I'll let the security libs experts comment on the rest of this change however. best regards, -- daniel - PR: https://git.openjdk.java.net/jdk/pull/2057
RFR: 8259662: SocketException should be passed through
Redo for 8237578: JDK-8214339 (SSLSocketImpl wraps SocketException) appears to not be fully fixed This also fixes JDK-8259516: Alerts sent by peer may not be received correctly during TLS handshake - Commit messages: - 8259662: SocketException should be passed through Changes: https://git.openjdk.java.net/jdk/pull/2057/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2057&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8259662 Stats: 488 lines in 7 files changed: 479 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/2057.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2057/head:pull/2057 PR: https://git.openjdk.java.net/jdk/pull/2057
