use fastdoubleparser jar

[pjfanning]

Relates to wrandelshofer/FastDoubleParser#25

Should lead to better performance on newer JDKs. Up until now, the code that was copied into jackson-core was optimised for Java8.

Shade the fastdoubleparser classes and bundle into jackson-core jar.

[cowtowncoder]

Ok, how much extra baggage would we incur for different JDKs? While I am no longer super-focused on jar size, I worry a bit about addition here.

I guess I should be able to build locally have a look, if I have time.

[pjfanning]

META-INF/versions/11 is a way to define override versions of classes that use features only available in Java 11 and above. Likewise for versions/17 and versions/19.

[cowtowncoder]

META-INF/versions/11 is a way to define override versions of classes that use features only available in Java 11 and above. Likewise for versions/17 and versions/19.

Ah ok makes sense.

I hope build can still be done with JDK 8 since classes included have been pre-compiled into jar. But I guess that's easy to find out.

[pjfanning]

Only a few classes are overridden in the META-INF/versions.

An alternative approach is to just use the fastdoubleparser as a transitive dependency that is marked as optional - so that users who want the feature will need to download the jar explicitly.

[cowtowncoder]

@pjfanning No I think that approach is fine, my only concern would be if we needed a later JDK for building (as long as we have JDK 8 baseline for jackson-core).

And I really like the idea of shading from existing jar.

[GedMarc]

yeah we had that with the moule-info's, cause they are compiled in version 9, animal-sniffer and the such highlight it was an error if it's not placed in the version that it's compiled in :( We had to move it from versions/11 to versions/9 for this reason -

So just make sure the class is compiled in the version you place the class in

[GedMarc]

And don't forget to set the Multi-Release flag to true in the Manifest file so it takes effect on JDK9 and up

[cowtowncoder]

I wonder if we should have GH Action to run simple tests against built jar with different JDKs (unit tests run against compiled classes, before package).
Although issues we've had tend to be on some more exotic set ups I guess, not with vanilla usage so maybe that would not find many issues.

[pjfanning]

Jackson Databind CI build could be said to cover testing the built jackson-core jar.

[cowtowncoder]

Jackson Databind CI build could be said to cover testing the built jackson-core jar.

I guess that's true, yes. Good point.

It would be nice to figure out a way to trigger cascading re-build/re-testing -- that's something I've thought about occasionally for years -- as that would expose issues when changes occur. But at least the way things are we should eventually find out problems, as long as jackson-databind gets commits often enough.

[cowtowncoder]

@pjfanning Let me know if and when this is ready; I finally got the stream-read-constraints thing fully merged in so I should be able to get this too.

[pjfanning]

@cowtowncoder this should be ok to merge now

I've tested the built jars with jackson-databind on my laptop and they seem to work fine.

[cowtowncoder]

Thank you @pjfanning !

[cowtowncoder]

Sidenote: never checked exactly how much size fast parsing adds: it almost DOUBLEs the size of jackson-core... ugh.

But that's regardless of shading, no change here.
I guess I'm happy I did not consider this earlier, would have had to consider it carefully; for some use cases this is unfortunate increase.

[cowtowncoder]

Merged to master without problems.

I think we'll get some verification of goodness from CI too, since tests are run against JDKs 8, 11 and 17

[cowtowncoder]

@pjfanning One interesting finding: after adding JDK 19 to CI, there are now fails for number parsing:

https://github.com/FasterXML/jackson-core/actions/runs/3580398897/jobs/6022492296

Not sure what gives, and whether these might be worth reporting to maintainer of fastdoubleparser?

I'll have to think of what to do wrt JDK 19; awkward to have build failures there, although it will not prevent SNAPSHOT publishing (those are done from JDK 8 build).

[pjfanning]

@pjfanning One interesting finding: after adding JDK 19 to CI, there are now fails for number parsing:

https://github.com/FasterXML/jackson-core/actions/runs/3580398897/jobs/6022492296

Not sure what gives, and whether these might be worth reporting to maintainer of fastdoubleparser?

I'll have to think of what to do wrt JDK 19; awkward to have build failures there, although it will not prevent SNAPSHOT publishing (those are done from JDK 8 build).

I added #849

The tests that are being removed by that PR as not related to fastdoubleparser jar. They relate to the double/float number writing as opposed to parsing.

[pjfanning]

Sidenote: never checked exactly how much size fast parsing adds: it almost DOUBLEs the size of jackson-core... ugh.

But that's regardless of shading, no change here. I guess I'm happy I did not consider this earlier, would have had to consider it carefully; for some use cases this is unfortunate increase.

It might still be worth considering taking the fastdoubleparser code out and having an optional dependency on the external jar. Likewise, the schubfach number writing code could be moved out to another jar and made an optional dependency.

[cowtowncoder]

While I'd like to reduce jar size, I think that complexity to users on optional components may not be worth it. It would add new failure modes too for possible version incompatibilities.
I am mostly concerned since so far it's been just 1 jar with 0 dependencies and since jackson-core is a dependency of almost everything in Jackson (excluding jackson-annotations). So any breakage or failure mode there has wide-ranging eeffects.

But we can definitely keep the possibility in mind in case it becomes actuality again.
Having fastdoubleparser shaded should make this option more practical (less work).