-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Bump com.fasterxml.jackson:jackson-bom from 2.17.0 to 2.17.1 #40584
Conversation
With my commit I have added a unit test, so I suggest to update it, to help capturing regressions on quarkus version which could observe different Jackson versions |
I've now see what Jackson does at https://github.com/FasterXML/jackson-core/blob/jackson-core-2.17.1/src/main/java/com/fasterxml/jackson/core/util/JsonRecyclerPools.java#L31-L41 so there won't be any allocation apart from https://github.com/FasterXML/jackson-core/blob/jackson-core-2.17.1/src/main/java/com/fasterxml/jackson/core/util/JsonRecyclerPools.java#L127 which is negligible (class loading apart, but that was happening in my pr as well!); thanks @mariofusco now that there is no default pool allocation, this one is way more robust! |
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.
LGTM thanks @mariofusco well done!
Status for workflow
|
@rsvoboda @aloubyansky marked for backport because of #41099 . |
This pull request bump Jackson from 2.17.0 to 2.17.1 replacing this automatically generated one.
In this release Jackson changed the default
RecyclerPool
implementation ( see FasterXML/jackson-core@2a4a6dc ) because of some problems we found in theLockFree
version. At the moment they decided to revert back to the ThreadLocal-based implementation, the non-friendly one for virtual thread, so very likely this will be changed again in the near future.For this reason I believe that the test checking for the default pool implementation currently in use by Jackson, that has been introduced with this commit, is brittle and I removed it. That test was necessary to check that the
VertxHybridPoolObjectMapperCustomizer
was behaving as expected, replacing theRecyclerPool
implementation only when the user hasn't explicitly configured a pool that is different from the default one. I changed theVertxHybridPoolObjectMapperCustomizer
so that it now compares theRecyclerPool
implementation in use with the Jackson's default one, thus making the test that I removed no longer necessary.As a side note it seems odd to me that Quarkus uses a version of
jackson-core
that is different from the one used by Vert.x, which is still depending on Jackson 2.16.1. Is this expected?/cc @franz1981 @gsmet @vietj @geoand