-
Notifications
You must be signed in to change notification settings - Fork 94
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
Disable Jackson field name interning #2583
Conversation
Interning introduces excessive contention FasterXML/jackson-core#946
Generate changelog in
|
My understanding, which I haven't validated, is that interning is applied atop canonicalization (so that writing/reading N objects will still reuse field names based on the same property as mapped from a field via data-binding) but not arbitrary |
My previous comment on this PR was off base, this doesn't have much to do with smile or id-based lookups that smile employs. Given the small size of the cache, I don't think we're any better off with the cache than without it, because many of our services define more parameter names than the intern-cache max-size. Where the intern cache may be providing us some benefit is through its recency bias, however I imagine that disappears when multiple operations execute in parallel anyhow. I suspect that it's an upstream bug that map keys are considered for interning, but regardless of whether that is resolved, I think we're going to be better off opting out of the feature given the cache size. The strings will be very short-lived in most cases (used to map to a method) and avoiding the cache may help hotspot escape analysis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding this, @schlosna !
Released 7.46.0 |
Thanks for the in depth test @carterkozak ! |
Before this PR
Seeing synchronization contention in
com.fasterxml.jackson.core.util.InternCache.intern(String)
via JFR with services processing high volumes of SMILE encoded data, where we are reusingObjectMapper
I don't believe we are failing to close theJsonParser
. See FasterXML/jackson-core#946After this PR
==COMMIT_MSG==
Interning introduces excessive contention
==COMMIT_MSG==
Possible downsides?
This potentially increases allocations and GC work necessary for deserialization.
Long term we are probably better off pushing String deduplication to the Garbage Collector via
-XX:+UseStringDeduplication
per palantir/sls-packaging#1378Relevant context:
https://shipilev.net/jvm/anatomy-quarks/10-string-intern/
G1 and Shenandoah GC support string deduplication as of JDK 17
Only enable on JDK 17+ due to fix https://bugs.openjdk.org/browse/JDK-8277981
JDK 18+ enable string deduplication for SerialGC, ParallalGC, and ZGC.
https://malloc.se/blog/zgc-jdk18