Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cowtowncoder opened this issue Jan 14, 2024 · 1 comment
Labels
2.17 Issues planned (at earliest) for 2.17 performance Issue related to performance problems or enhancements

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 14, 2024

(based on discussion FasterXML/jackson#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 cowtowncoder added performance Issue related to performance problems or enhancements 2.17 Issues planned (at earliest) for 2.17 labels Jan 14, 2024
@cowtowncoder cowtowncoder changed the title BufferRecycler should avoid setting replacement if one already returned BufferRecycler should avoid setting replacement if one already returned, bigger Jan 14, 2024
@cowtowncoder
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.17 Issues planned (at earliest) for 2.17 performance Issue related to performance problems or enhancements
Projects
None yet
Development

No branches or pull requests

1 participant