Optimize generated Jackson deserialization - #3029
Conversation
Generate changelog in
|
schlosna
changed the title
✅ Successfully generated changelog entry!Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. 📋Changelog Preview💡 Improvements
|
kunalrkak
left a comment
kunalrkak
left a comment
There was a problem hiding this comment.
commented on the generated code itself, ignored the actual codgen piece for now
| } | ||
| } | ||
|
|
||
| static final class Deserializer extends JsonDeserializer<SimpleUnion> implements ResolvableDeserializer { |
There was a problem hiding this comment.
what benefit does ResolvableDeserializer give us here?
| static final class Deserializer extends JsonDeserializer<SimpleUnion> implements ResolvableDeserializer { | ||
| private static final Class<?>[] VARIANT_TYPES = new Class<?>[] {Foo.class, Bar.class, Baz.class}; | ||
|
|
||
| private volatile JsonDeserializer<?>[] deserializers; |
There was a problem hiding this comment.
nit:
| private volatile JsonDeserializer<?>[] deserializers; | |
| private volatile JsonDeserializer<?>[] variantDeserializers; |
| } | ||
| } | ||
|
|
||
| static final class Serializer extends JsonSerializer<SimpleUnion> { |
There was a problem hiding this comment.
is this different than what Jackson does out of the box? i.e. why does this need to be custom
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| @JsonDeserialize | ||
| @JsonSerialize |
There was a problem hiding this comment.
why do we need these extra annotations on each subvariant?
|
|
||
| private static boolean isTypeField(String fieldName, DeserializationContext context) { | ||
| return "type".equals(fieldName) | ||
| || (context.isEnabled(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) |
There was a problem hiding this comment.
nit: given this flag is constant can we look it up once and pass it into this method?
| JsonDeserializer<?>[] updated = deserializers.clone(); | ||
| updated[variantIndex] = deserializer; | ||
| deserializers = updated; |
There was a problem hiding this comment.
why are we handling for a race condition here (shouldn't findRootValueDeserializer be idempotent?)
There was a problem hiding this comment.
also why this particular method? I'm comparing it to the jackson code for polymorphic deser and don't see it used, but do see findContextualValueDeserializer used here:
https://github.com/FasterXML/jackson-databind/blob/jackson-databind-2.21.1/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsPropertyTypeDeserializer.java#L217
2 participants
Summary
Performance
Midpoints from two JVM forks with seven measured trials per fork:
Cold reader creation plus first read for the 101-variant union improved from about 490 microseconds and 134 KB to 300 microseconds and 22 KB. Serialization allocation remained unchanged; timing did not show a consistent shift.
The generated 101-variant deserializer decreased from 26,191 to 21,214 bytes compared with the initial reviewed implementation.
Testing
The server verifier test is excluded because the local native verifier requires unavailable libssl.so.1.1.