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

Performance optimization in CloseableThreadContext.closeMap() #2292

Closed
jengebr opened this issue Feb 15, 2024 · 3 comments
Closed

Performance optimization in CloseableThreadContext.closeMap() #2292

jengebr opened this issue Feb 15, 2024 · 3 comments
Labels
api Affects the public API
Milestone

Comments

@jengebr
Copy link
Contributor

jengebr commented Feb 15, 2024

Description

Excess work is performed in CloseableThreadContext.closeMap() that shows up as a production hotspot in our client-facing, latency-sensitive application. Specifically, the use of multiple calls to ThreadContext.remove() and ThreadContext.put() causes multiple copy-and-modify operations in the underlying ThreadContextMap. The attached java files compare the performance of the original code vs. a modification that groups keys by remove vs. replace, and makes zero or one calls to ThreadContext.remove() and ThreadContext.put().

On my system, tests with thread locals disabled (Constants.ENABLE_THREADLOCALS == false) show a 40% improvement:

Done with old in 521
Done with old in 535
Done with old in 537
Done with old in 516
Done with old in 516
Done with new in 284
Done with new in 285
Done with new in 284
Done with new in 301
Done with new in 292

With thread locals enabled (Constants.ENABLE_THREADLOCALS == true) show a 20% improvement:

Done with old in 386
Done with old in 388
Done with old in 386
Done with old in 389
Done with old in 408
Done with new in 308
Done with new in 309
Done with new in 307
Done with new in 307
Done with new in 313

The impact of the thread locals flag is because of the different implementations of ThreadContextMap. However, both cases demonstrate substantial improvement.

Configuration

Version: 2.20

Operating system: Affects all - tested on Linux and Windows

JDK: Tested on OpenJDK 8 and OpenJDK 17

Logs

CloseableThreadContextSpeedTest.java.txt

Reproduction

See attached file.

@ppkarwasz
Copy link
Contributor

@jengebr, can you submit a PR?

@jengebr
Copy link
Contributor Author

jengebr commented Feb 16, 2024

As requested: #2296

Hope I got the details right....

@vy vy added this to the 2.24.0 milestone Feb 19, 2024
@vy vy added the api Affects the public API label Feb 19, 2024
@vy
Copy link
Member

vy commented Feb 19, 2024

Fixed in #2296.

@vy vy closed this as completed Feb 19, 2024
@ppkarwasz ppkarwasz modified the milestones: 2.24.0, 2.23.1 Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the public API
Projects
None yet
Development

No branches or pull requests

3 participants