search

FasterXML / jackson-core

Core part of Jackson that defines Streaming API as well as basic shared abstractions
Apache License 2.0
2.27k stars 795 forks source link

`BufferRecycler` should avoid setting replacement if one already returned, bigger #1186

Closed cowtowncoder closed 10 months ago

cowtowncoder commented 10 months ago

(based on discussion https://github.com/FasterXML/jackson/discussions/204 -- good suggestion by @kkkkkhhhh)

It seems that BufferRecycler will always replace assigned buffer when release method is called. But it would probably make sense to only replace null or smaller buffer, and avoid replacing bigger buffer with smaller. While in the original expected usage sequence should always be "alloc / release / allow / release" (in which case "release" would be replacing null), there can be cases where this does not hold (multiple parsers/generators per thread, concurrently; but also just parser-and-generator case).

So let's add some basic checking into release method.

cowtowncoder commented 10 months ago

Note: in multiple-allocation case there is no real guarantee that the oldest buffer would be retained. Fix just helps to ensure the longest buffer chunk is retained.

But instead of trying to retain oldest buffer at this level we should migrate to newer RecyclerPool implementations (ones not based on ThreadLocal) as they will also better support multiple-instances-of-buffer-per-thread case, in addition to other benefits.