Fix ZStandard native memory leaks

[lorban]

Fixes #14028 by closing the zstd context when sources / sinks are released or when the GC cleans them up.

Also add buffer pooling to the zstd streams.

[joakime]

Simple enough.

[joakime]

I don't like the reduction in recommended default size.

Can we instead have a log warning be produced if the bufferPool isn't setup properly to allow for zstandard lib recommendations?

[joakime]

This size is smaller than the recommended size by the zstandard lib.

[joakime]

This size is smaller than the recommended size by the zstandard lib.

[lorban]

@joakime we currently have no API to detect the maximum pooled buffer size, so if we were to warn "please make sure your pool is properly configured" we'd always do it, even when the pool is properly configured.

I hardcoded 64 KB here because the default maximum pooled buffer size is 64 KB, and it is the only solution that is both quick and easy to do that I could think of.

If zstd were to use non-pooled buffer, the performance would go down the drain: each direct byte buffer object has to be finalized (requires enqueuing/dequeuing from a concurrent queue, expensive!) and each time a new buffer is allocated it has to be zero'ed (filling 132 KB of memory with zeros, expensive!).

I agree this solution isn't ideal, but the only alternative I can come up that doesn't require modifying the ByteBufferPool interface with is to change the default ArrayByteBufferPool implementation to pool up to 256 KB buffers, i.e.: the previous ceiling value.

@sbordet @gregw opinions?

[sbordet]

Reading https://datatracker.ietf.org/doc/html/rfc8878#section-3.1.1.1.2, the min window size can be just 1 KiB.
However, there is a recommendation for 8 MiB, which seems just huge.

I think it's best for us to stick with a smaller size like 64 KiB, and document the implications with the ByteBufferPool.

[lorban]

So I've reverted to capping the zstd buffer sizes to 64 KB, and I added a comment to the javadoc explaining that this setting has implications on the ByteBufferPool configuration.

I believe this is the most reasonable thing to do for now as a user can always manually set the buffer size.

[joakime]

The zstandard requirements are Window_Size + 2 * Block_Maximum_Size

Of which, ...

• Window_Size defaults to 128 KB.
• Block_Maximum_Size = min(128 KB, Window_Size).

https://github.com/facebook/zstd/wiki/Using-libzstd-in-a-memory-constrained-environment

[joakime]

We use the zstd-jni which is just a Java JNI layer on top of the facebook zstandard lib.
That lib has updated real-world defaults of 128 KB for Window Size.

It recommends using 128KB for the Window Size if dealing with sub 2GB inputs on non-streaming modes (we don't use the streaming modes).
It recommends higher Windows Size of up to 8MB if dealing with inputs over 2GB.

[lorban]

@joakime @sbordet I've reverted the code changes related to the zstd buffer size and left the issue as-is for now, with only changes to the javadoc to hint at the problem.

This isn't a regression as this problem exists since the introduction of zstd (de)compression and we need the other fixes to be included in the next release that has to be cut this week, so let's postpone the buffer problem to the next release and simply ignore it for now.

[joakime]

The current PR is invalid for zstandard, and will break it.

[joakime]

Good, for now.
We need a new issue to track the ByteBufferPool changes to make it expose it's limits.

?