Repository navigation
Reuse the wrapped type's deserializer in value class deserializers (#1250) - #1251
Open
sandrosonntag wants to merge 1 commit into
Open
sandrosonntag wants to merge 1 commit into
sandrosonntag wants to merge 1 commit into
Conversation
The value class deserializers called `p.readValueAs(inputType)` for every value. In 3.x this ends in `DeserializationContext.readValue()`, which looks up the root deserializer without caching it, so non-cachable deserializers (`BigDecimal`, `UUID`, `java.time`, ...) were constructed again for every value under the DeserializerCache lock. Look the deserializer up once on first use and reuse it; null and missing-token handling stays the same as in `readValue()`. Fixes FasterXML#1250
sandrosonntag
force-pushed
the
fix/value-class-delegate-deserializer-cache
branch
from
October 27, 2026 05:14
dc4b023 to
60a3076
Compare
Author
|
I have sent the signed CLA (CLA-jackson-2026) to cla@fasterxml.com today. |
This branch has not been deployed
This file contains hidden or 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
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.
Fixes #1250
Problem
WrapsAnyValueClassBoxDeserializer,HasConversionCreatorWrapsSpecifiedBoxDeserializerandNoConversionCreatorBoxDeserializercallp.readValueAs(inputType)for every value. In 3.x that goes toDeserializationContext.readValue(), which looks up the root deserializer without caching it. Non-cachable deserializers (BigDecimal,UUID,java.time, ...) are therefore constructed again for every value, under theDeserializerCachelock. Details are in #1250.Change
WrappedValueReader: it looks up the deserializer of the wrapped type withctxt.findRootValueDeserializer(...)on first use, keeps it in a@Volatilefield and reuses it after that. Null handling and the "no current token" handling are the same as inDeserializationContext.readValue().NoConversionCreatorBoxDeserializercovers theInt/Long/String/UUIDcases, which took the same path. Its subclasses now passinputTypeto the constructor instead of overriding an abstract property.@JsonFormaton a value class property still does not reach the wrapped type, same as before. The lookup is still lazy, so it (and any error it reports) only happens when a value is actually read.ctxtpassed todeserialize()instead ofp.objectReadContext(), like other deserializers do. With normal reading the two are the same object.Tests
GitHub1250:wrappedDeserializersAreNotConstructedPerValuecounts constructions with aValueDeserializerModifier. Without the fix the second read builds them all again (BigDecimal=40→80, LocalDate=20→40, UUID=10→20), so the test fails. With the fix nothing is built on the second read, and the test passes.valuescoversBigDecimal,LocalDate,UUID(WrapsJavaUuid), a@JsonCreatorwith conversion, a nullable wrapped type,nullproperty and list elements, andList/Mapcontents.recursiveWrappedTypecovers a value class whose wrapped bean refers back to the value class../mvnw clean verify: 739 tests, 0 failures, 6 skipped (the same 6 are skipped on3.2without the change).Measurement
I used the reproducer from #1250: 1000 items, an
ObjectReadershared by N threads, items/s summed over all threads, 3 s warm-up and 5 s measurement, no JMH. "Before" is the3.2branch and "after" is this branch, both on databind 3.2.4. It ran on an Apple M2 Pro with JDK 21.0.5 on a busy laptop, so the absolute numbers are rough. A second run in reverse order showed the same pattern.Deserializer constructions during the second read: before
{BigDecimal=1000, LocalDate=1000}, after{}.Notes
3.2because the README asks for bugfixes against the current stable version to go there.src/mainis identical on3.2and3.x, and I'm happy to retarget. 2.x is not affected, becausereadValueAs()goes through the codec's root deserializer cache there.release-notes/. I can add entries if you want them.