FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
310 stars 133 forks source link

use String constructor directly for short ascii values #344

Closed brharrington closed 1 year ago

brharrington commented 1 year ago

On jdk9+ the String internals changed and it is more efficient to just use the String contructor that takes a byte array when dealing with ASCII values. Since jdk11 is the most recent LTS version with these changes and there are some methods added to String that can be used to detect it, this change uses the String constructor directly when on a newer JDK. The old behavior is preserved for jdk8.

Fixes #342

brharrington commented 1 year ago

From my tests it seems to maintain roughly the same performance on jdk8 and shows improvements for jdk11+.

Benchmark                       Mode  Cnt       Score       Error   Units

java 8 baseline                thrpt    5  348058.136 ± 29095.348   ops/s
java 8 with change             thrpt    5  342844.462 ± 32386.599   ops/s

java 11 baseline               thrpt    5  313255.865 ± 43280.554   ops/s
java 11 with change            thrpt    5  347304.547 ± 44594.978   ops/s

java 17 baseline               thrpt    5  322069.060 ± 28697.702   ops/s
java 17 with change            thrpt    5  427788.567 ± 25312.986   ops/s
cowtowncoder commented 1 year ago

Looks good, and I think can be merged in 2.14 for 2.14.1 patch.

One thing I may need: if we don't yet have a CLA (I feel we might have but didn't see one?), it'd be from here:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

and the usual way is to print it out, fill & sign, scan/photo, email to info at fasterxml dot com. Once we have it (please LMK if you did send it earlier and I'll try to locate it) I can go ahead merge this nice improvement in.