Remove Netty UnpooledByteBufAllocator workaround

[olivergillespie]

Motivation and Context

If using the JDK SSL provider with Netty, we currently switch away from Netty's default ByteBuffer allocator to the unpooled allocator, to work around a regression in Netty 4.1.43 -
netty/netty#9768.

The unpooled allocator causes high allocation rate, increasing GC frequency and reducing application throughput.

The Netty issue has been fixed in later versions, so we should revert back to using the default.

Fixes #6843

Modifications

This is essentially a revert of the relevant parts of a72f494ac1fb. Don't override the buffer allocator, and remove the wiring that was added just to allow that. Also remove the test that only tests that behaviour, no longer useful. Now, whatever the environment default allocator is will be respected.

Testing

./mvnw clean install -pl :netty-nio-client --am

Benchmark Results

java -jar test/sdk-benchmarks/target/benchmarks.jar ".*NettyHttpClientH1Benchmark.*" -prof gc -f 1 -wi 3 -i 5

We can see huge allocation rate reduction along with some throughput improvement when using JDK provider, and no effect when using the OpenSSL provider, as expected.

sslProvider = JDK

Benchmark	Before	After	Change
concurrentApiCall alloc (B/op)	164,715 ± 114	96,240 ± 402	-41.6%
sequentialApiCall alloc (B/op)	164,764 ± 46	96,260 ± 384	-41.6%

sslProvider = OpenSSL

Benchmark	Before	After	Change
concurrentApiCall (ops/s)	9,559 ± 206	9,517 ± 247	-0.4%
concurrentApiCall alloc (B/op)	90,260 ± 165	89,678 ± 512	-0.6%
sequentialApiCall (ops/s)	2,780 ± 153	2,793 ± 246	+0.5%
sequentialApiCall alloc (B/op)	89,704 ± 36	89,555 ± 25	-0.2%

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• My change requires a change to the Javadoc documentation
• I have updated the Javadoc documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
• My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

• I confirm that this pull request can be released under the Apache 2 license

[RanVaknin]

Hi @olivergillespie ,

Thanks for the high quality PR.

Can you share a little bit of info about the JVM version, and machine architecture you are running the JMH benchmarks on?

We usually run benchmarks with at least 2 forks to minimize noise. Can you please rerun those benchmarks with additional forks?

Additionally you mentioned

We can see huge allocation rate reduction along with some throughput improvement when using JDK provide

Do you have any benchmarks that show the increase in throughput?

All the best,
Ran~

[olivergillespie]

Thanks @RanVaknin. My original results were with Corretto 17.0.19 on x86 (AWS EC2 m5.4xl).

Allocation per op is usually not noisy and doesn't vary fork to fork or across architectures, nor JVM version (in most cases, compiler optimizations and object layout can vary but since the key here is large byte buffers it won't make a difference) but for completeness I've run again with 5 forks on both x86 (Corretto 17.0.19m m5.4xl) and aarch64 (Corretto 21.0.8, m6g.xl). The improvement holds.

The same benchmarks are what (initially) showed the throughput improvement - sequentialApiCall op/s increased by 8% - but seems like this was just noise, it's not reproduced in these runs with more forks. I've updated the description to remove that part, it's not the point of this change anyway.

java -jar test/sdk-benchmarks/target/benchmarks.jar ".*NettyHttpClientH1Benchmark.*" -prof gc -f 5 -wi 3 -i 3 -p sslProviderValue=jdk

Throughput (ops/s)

Benchmark	x86 Corretto 17 Before	x86 Corretto 17 After	% Change	aarch64 Corretto 21 Before	aarch64 Corretto 21 After	% Change
concurrentApiCall	9634 ± 240	9547 ± 282	insignificant	5166 ± 161	5222 ± 124	insignificant
sequentialApiCall	2756 ± 87	2772 ± 67	insignificant	1626 ± 75	1765 ± 38	insignificant

Allocation Rate (B/op)

Benchmark	x86 Corretto 17 Before	x86 Corretto 17 After	% Change	aarch64 Corretto 21 Before	aarch64 Corretto 21 After	% Change
concurrentApiCall	164973 ± 241	96406 ± 351	-42%	161353 ± 495	93200 ± 379	-42%
sequentialApiCall	164778 ± 200	96184 ± 308	-42%	161357 ± 237	92730 ± 391	-43%

[olivergillespie]

Merged in #6968, thanks!