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

Fix #1114: make BufferRecyclerPool generic #1115

Merged
merged 7 commits into from
Sep 28, 2023

Conversation

cowtowncoder
Copy link
Member

@cowtowncoder cowtowncoder commented Sep 26, 2023

PR to implement #1114 by rewriting BufferRecyclerPool, implementations so that there are:

  • Abstract base types for recycling any types (not just BufferRecycler)
  • JsonFactory-specific subtypes, implementation classes that pool BufferRecycler values.

Will add new container class JsonBufferRecyclers for latter, keep BufferRecyclerPool for former.
Will not yet rename BufferRecyclerPool but may consider it as follow up.

@cowtowncoder cowtowncoder added the 2.16 Issue planned (at earliest) for 2.16 label Sep 26, 2023
@@ -262,7 +262,7 @@ public static int collectDefaults() {
/**
* @since 2.16
*/
protected BufferRecyclerPool _bufferRecyclerPool;
protected BufferRecyclerPool<BufferRecycler> _bufferRecyclerPool;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will probably consider renaming of BufferRecyclerPool as it'll allow recycling other things, but initially leaving name as-is to simplify merging to master.

*
* @since 2.16
*/
public final class JsonBufferRecyclerPools
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will move BufferRecycler-bound implementations here next.

@cowtowncoder cowtowncoder marked this pull request as ready for review September 28, 2023 04:07
@cowtowncoder cowtowncoder merged commit 2640def into 2.16 Sep 28, 2023
5 checks passed
@cowtowncoder cowtowncoder deleted the tatu/2.16/1114-generic-recycler-pool branch September 28, 2023 04:12
@mariofusco
Copy link
Contributor

@cowtowncoder I saw what you did and I agree with this change.I also originally implemented the pool in a generic way because I "felt" it was the right thing to do, but then I reverted it.

I also checked that this works fine with the quarkus integration that I'm attempting. By the way we would need the stable 2.16 asap in order to start using the pool also on our side, do you have any on when it will be released?

@cowtowncoder
Copy link
Member Author

@mariofusco I have been about to send an email about 2.16 plans. Things are quite close to getting RC1 out; I just have one feature I really want to get in jackson-databind before that. So if all went well, could get RC out in 2-3 weeks. And this time around only doing one -- seems like few devs really use release candidates anyway. RC1 should have recycler pool thing finalized wrt API (unless issues were found from within).

Also: I (or someone else) need to work on using now-generic pools to replace existing ThreadLocal based hard-coded reuse on Smile, Avro and... JAX-RS module? Ideally would tackle for 2.16.

@cowtowncoder
Copy link
Member Author

Making good progress on "last" issue to resolve (databind , then FasterXML/jackson-databind#4096); email sent.
But maybe I'll get to doing RC1 during next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16 Issue planned (at earliest) for 2.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants