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

Change default recycler pool to newConcurrentDequePool() in 2.18 #1266

Closed
cowtowncoder opened this issue Apr 19, 2024 · 3 comments
Closed

Change default recycler pool to newConcurrentDequePool() in 2.18 #1266

cowtowncoder opened this issue Apr 19, 2024 · 3 comments
Labels
2.18 Issues planned at earliest for 2.18

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Apr 19, 2024

(note: follow-up to #1256)

Altho we will revert back to ThreadLocalPool for 2.17.1 and rest of 2.17.x, we do want to move away in 2.18.
Let's use "ConcurrentDequePool" (via JsonRecyclerPools.newConcurrentDequePool()) as that seems to behave better and have otherwise similar approach as shared-lock-pool, but without potentially failing acquire that can lead to imbalanced handling & ever-growing pool.

@cowtowncoder cowtowncoder added the 2.18 Issues planned at earliest for 2.18 label Apr 19, 2024
@cowtowncoder cowtowncoder changed the title Change default recycler pool to concurrentDequePool in 2.18 Change default recycler pool to bewConcurrentDequePool() in 2.18 Apr 20, 2024
@cowtowncoder
Copy link
Member Author

Actually. Given all the feedback received so far, it appears that the ThreadLocal-based pool still performs best for pre-Loom threading. So let's postpone this change to 3.0 (master); will revert this change

@pjfanning pjfanning changed the title Change default recycler pool to bewConcurrentDequePool() in 2.18 Change default recycler pool to newConcurrentDequePool() in 2.18 Jun 4, 2024
@marlonpatrick
Copy link

ThreadLocal-based pool still performs best for pre-Loom threading.

@cowtowncoder I would really appreciate it if you could clarify some doubts for me:

1- What about an application using virtual threads? Did you notice any problems with newConcurrentDequePool() when used in conjunction with virtual threads?

2 - In the 3.0 release, you think the default will be newConcurrentDequePool() or you are considering another alternative?

3 - For Jackson 2.17.x, is newConcurrentDequePool() the recomended approach with virtual threads?

@cowtowncoder
Copy link
Member Author

I have no experience on virtual threads myself, unfortunately: I hope others (like @mariofusco ) can comment here. With that:

  1. Cannot comment
  2. Not certain yet but that is my inclination
  3. I think so, but have not really seen much information on people actually running with it.

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

No branches or pull requests

2 participants