Carrier Thread pinning on stack CommittingOutputStream.flushBuffer()

[vasanth-bhat]

Carrier Thread pinning on stack CommittingOutputStream.flushBuffer()

[jansupol]

I have copied the ByteArrayOutputStream into a new file and replaced the synchronized with the ReentrantLock. The synchronized blocks are 10x faster.

[jansupol]

At first glance, adaptedOutput.write(buffer.toByteArray()); seems worse performance-wise as it is synchronized too and it creates a copy of the data buffer for each call. We should measure the performance and have the change for JDK 21+ only, if worse on JDK [8,20)

[vasanth-bhat]

At first glance, adaptedOutput.write(buffer.toByteArray()); seems worse performance-wise as it is synchronized too

Yes, BufferedOutputStream.toByteArray() is synchronised , but it does not lead to the specific issue of carrier thread pinning, as there is no IO happening from with this sync block.

https://github.com/openjdk/jdk/blob/jdk-21%2B35/src/java.base/share/classes/java/io/ByteArrayOutputStream.java

public synchronized byte[] toByteArray() {
return Arrays.copyOf(buf, count);
}

yes, It does make Array copy .

Looking further at the. stack snippet. in this scenario, the concrete type of "adaptedOutput" is of. type. "TracingObserver$TracingStreamOutputDelegate" .

https://github.com/helidon-io/helidon/blob/e93ee0c0c9b8762212a7f07cb473fec6bc52d827/webserver/observe/tracing/src/main/java/io/helidon/webserver/observe/tracing/TracingObserver.java#L490

Further, The BufferedOutputStream.write() is synchronized only when it is subclassed and not used directly.
https://github.com/openjdk/jdk/blob/890adb6410dab4606a4f26a942aed02fb2f55387/src/java.base/share/classes/java/io/BufferedOutputStream.java#L87

Stack Snippet

io.helidon.webserver.http1.Http1ServerResponse$BlockingOutputStream.write(Http1ServerResponse.java:585) io.helidon.webserver.http1.Http1ServerResponse$BlockingOutputStream.write(Http1ServerResponse.java:470) java.base/[java.io](http://java.io/).BufferedOutputStream.implWrite(BufferedOutputStream.java:217) java.base/[java.io](http://java.io/).BufferedOutputStream.write(BufferedOutputStream.java:200) io.helidon.webserver.http1.Http1ServerResponse$ClosingBufferedOutputStream.write(Http1ServerResponse.java:749) io.helidon.webserver.observe.tracing.TracingObserver$TracingStreamOutputDelegate.write(TracingObserver.java:525) java.base/[java.io](http://java.io/).ByteArrayOutputStream.writeTo(ByteArrayOutputStream.java:163) <== monitors:1 org.glassfish.jersey.message.internal.CommittingOutputStream.flushBuffer(CommittingOutputStream.java:278) org.glassfish.jersey.message.internal.CommittingOutputStream.commit(CommittingOutputStream.java:232)

[jansupol]

Interesting test results, for buffer=ByteArrayOutputStream, adaptedOutput=FileOutputStream, 32MB written:

|    JDK     |     Actual  Method    |      Proposed Method      |
| -------------------------------------------------------------- |  
|     8      |        90ms           |              98ms         |
|     11     |        35ms           |              40ms         | 
|     17     |        45ms           |              50ms         |
|     21     |        43ms           |              48ms         | 

It looks like the proposed method is about 10% slowlier, no virtual threads. So we should have this just for virtual threads.

[jansupol]

Perhaps a better solution is to have a ByteArrayOutputStream without synchronized blocks, with reentrant locks instead to avoid data copying.

[jansupol]

#5629 Is for 2.x and 3.0, we should check for Thread#isVirtual in 3.1 and forward.

[jansupol]

Keep open for 3.1

[vasanth-bhat]

#5632 fixes the issue in 2.4.3.

Helidon 4.x. uses jersey 3.1.5
https://github.com/helidon-io/helidon/blob/main/dependencies/pom.xml
<version.lib.jersey>3.1.5</version.lib.jersey>

Which version of 3.1 would have this fix for Helidon to uptake?

[jansupol]

The next one, 3.1.7