Large responses written slowly due to allocating too small buffers

[ppalaga]

fix #1066

[ppalaga]

@franz1981 @geoand I tried to implement your suggestions here: I have taken the latest ResteasyReactiveOutputStream from Quarkus and adapted it to work here as VertxServletOutputStream. I did very similar edits like @dufoli who ported it originally.

Porting the part with LazyResponse was not easy so I have omitted it for now. Is this perhaps what you meant with "jackson approach mentioned earlier"?

I did some benchmarks and the performance on my machine is great:

• Before 7954 ms
• After 838 ms

[ppalaga]

Added comments explaining from where the two new classes originate.

[franz1981]

LGTM, my only suggestion is related the tests; to avoid being captured by another regression, in the future, adding a test which ensure we are same or better than the expected number of packets, for a large write, would be great

[ppalaga]

9714f01: added the test ensuring that there is an expected number of chunks.

[ppalaga]

The way how I implemented the counting of chunks in ChunkedTest through overriding HTTP Client classes is all but elegant. Anybody knows of a nicer approach?

[ppalaga]

6da774a: made the CodeQL happy.

[franz1981]

These tests are never easy to be implemented...
So, you can do in different ways, actually:

• how you've done
• using an existing proxy which can count the http chunks coming from the server
• mocking some class on the server and verify how http data is sent through it
• using Netty's Embeddable Channel and intercept how vertx send buffers to it

As you can see there are no elegant one, just some could make uses of existing bits to shorten the amount of code and appear better, because the ugliness is well confined elsewhere :P

[ppalaga]

Thanks for the suggestions, @franz1981! I think it would be better to avoid changing anything on the server so that we do not influence other tests (which are also run in native mode) in this module. I am not finding any usable proxy implementation.

Thus, I am going to merge like this.

[ppalaga]

Open Questions:

1. You mentioned somewhere that it would be good to make the org.jboss.resteasy.reactive.server.vertx.AppendBuffer public, right? However, depending on resteasy-reactive-vertx here in Quarkus CXF would not be great. Can you perhaps see some module, from where we could re-use it both here and in resteasy-reactive-vertx? Somewhere in Vert.x maybe?
2. What's your opinion on creating a common parent for io.quarkiverse.cxf.transport.VertxServletOutputStream and org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveOutputStream? IMO it would be great to have it so that Quarkus CXF does not miss any future fixes.

[franz1981]

Let me route these to @geoand which have a much better knowledge of quarkus's module deps, and I can help doing it

[geoand]

Can you perhaps see some module, from where we could re-use it both here and in resteasy-reactive-vertx ?

Very unlikely

What's your opinion on creating a common parent for io.quarkiverse.cxf.transport.VertxServletOutputStream and org.jboss.resteasy.reactive.server.vertx.ResteasyReactiveOutputStream ?

I really don't want such kind of coupling

[ppalaga]

Fair enough, thanks for the clear statement, @geoand

[geoand]

🙏