Added type match check to read functions #937
Conversation
There was a problem hiding this comment.
@cowtowncoder @JooHyukKim
Could you please help me with a review of the exceptions to be thrown?
src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt
Outdated
Show resolved
Hide resolved
| inline fun <reified T> ObjectMapper.readValue(jp: JsonParser): T = readValue(jp, jacksonTypeRef<T>()) | ||
| inline fun <reified T> ObjectMapper.readValues(jp: JsonParser): MappingIterator<T> = readValues(jp, jacksonTypeRef<T>()) | ||
| .checkTypeMismatch() |
There was a problem hiding this comment.
Originally, there is an overhead of generating a TypeReference each time, and this change adds another overhead of checking.
Therefore, after checking benchmarks, I will add a document to avoid using it in situations where performance is important.
There was a problem hiding this comment.
If performance is issue.... would simple try-catch instead of this type check(this PR) have better performance?
There was a problem hiding this comment.
First, I confirmed that there was already an 8% drop in throughput before the change.
https://github.com/k163377/read-value-benchmark/blob/master/reports/results.csv
The throughput is high enough to begin with, so I don't think the performance of this function will be an issue in basic use cases.
However, I am considering giving only comments for use cases that require extreme performance.
There was a problem hiding this comment.
After a little more consideration, I decided not to make a performance statement because it is unlikely that the overhead of this function will actually dominate.
371a858
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (6)
- src/main/kotlin/com/fasterxml/jackson/module/kotlin/Extensions.kt: Language not supported
- src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValueTest.kt: Language not supported
- src/test/kotlin/com/fasterxml/jackson/module/kotlin/ReadValuesTest.kt: Language not supported
- src/test/kotlin/com/fasterxml/jackson/module/kotlin/TestCommons.kt: Language not supported
- src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/WithoutCustomDeserializeMethodTest.kt: Language not supported
- src/test/kotlin/com/fasterxml/jackson/module/kotlin/kogeraIntegration/deser/valueClass/deserializer/SpecifiedForObjectMapperTest.kt: Language not supported
k163377
changed the title
Fixed a problem with functions such as
readValuethat could returnnulleven if non-null was specified inKotlin.This fix will close #399 .