Opt out of the TypeFactory cache until we can resolve cache contention #2595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FasterXML/jackson-databind#3876 FasterXML/jackson-benchmarks#5 The linked benchmarks show a small advantage of caching over not caching in most cases, in the best case for a cache: when a single operation is repetedly executed. In practice, the cache is used for all operations and sees heavy evictions, where contention is more likely and has greater impact on users. Given the hash collision issues that we're aware of, and the impacts we've observed, we will remove the cache until a version is available which isn't impacted by the linked issue for performance that's more predictable.
Generate changelog in
|
schlosna
approved these changes
Apr 13, 2023
|
|
||
| @Override | ||
| public NonCachingTypeFactory withModifier(TypeModifier mod) { | ||
| return new NonCachingTypeFactory(_parser, computeModifiers(_modifiers, mod), _classLoader); |
There was a problem hiding this comment.
Meta-non-actionable observation: the _modifiers and _classloader prefixed with _ threw me at first as I forgot that's the Jackson protected field style.
|
Based on the benchmarks and profiles I have seen, this tradeoff seems right for now to avoid the major contention and ConcurrentHashMap bin traversals on high throughput code paths. |
bulldozer-bot
bot
deleted the
ckozak/opt_out_of_jackson_typefactory_cache
branch
|
Released 7.50.0 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR
FasterXML/jackson-databind#3876
FasterXML/jackson-benchmarks#5
The linked benchmarks show a small advantage of caching over not caching in most cases, in the best case for a cache: when a single operation is repeatedly executed. In practice, the cache is used for all operations and sees heavy evictions, where contention is more likely and has greater impact on users.
Given the hash collision issues that we're aware of, and the impacts we've observed, we will remove the cache until a version is available which isn't impacted by the linked issue for performance that's more predictable.
After this PR
==COMMIT_MSG==
Opt out of the TypeFactory cache until we can resolve cache contention
==COMMIT_MSG==
Possible downsides?
No caching may result in worse performance in the best cases, but avoids cross-thread contention on overloaded buckets which result in exceedingly long operations.