woo-j / OkapiBarcode

Open-source barcode encoding program written in Java
http://www.okapibarcode.org.uk
Apache License 2.0
327 stars 97 forks source link

Performance: Lazy load `Charset` in `EciMode` #107

Open uwolfer opened 7 months ago

uwolfer commented 7 months ago

In my profilings, Symbol#eciProcess calling Charset#forName shows up quite slow (50ms). With this optimization, it no longer shows up, thusPdf417#encode goes down from 190ms to 140ms.

Before: Screenshot_20240315_233209

After: Screenshot_20240315_233439

codecov-commenter commented 7 months ago

Codecov Report

Attention: Patch coverage is 92.85714% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 87.26%. Comparing base (1a8bfa2) to head (6be3c80).

Files Patch % Lines
...rc/main/java/uk/org/okapibarcode/util/EciMode.java 92.15% 1 Missing and 3 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #107 +/- ## ============================================ + Coverage 87.25% 87.26% +0.01% Complexity 4299 4299 ============================================ Files 68 68 Lines 13934 13938 +4 Branches 3254 3254 ============================================ + Hits 12158 12163 +5 + Misses 1126 1125 -1 Partials 650 650 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gredler commented 7 months ago

I'm not sure this is worth the hassle, this initialization only happens once in the lifetime of the VM, right? Were you benchmarking a single call to createStructuredAppendSymbols?

uwolfer commented 7 months ago

I'm not sure this is worth the hassle, this initialization only happens once in the lifetime of the VM, right? Were you benchmarking a single call to createStructuredAppendSymbols?

Yes, I think this only is expensive once per VM lifetime.

Remark: In my tax use-case, the reference implementation is a Java CLI tool, which starts once per tax report PDF. That means, this initialization happens once per PDF. It is not too much of course, but in case you generate thousands of PDFs it still sums up a bit.

If you feel like it blows up the code too much, I am also OK with closing this PR.

gredler commented 7 months ago

Understood -- let's keep this one open as a future option, but I'm not sure it's worth it right now. By the way, have you tried Graal for this use case? It might be a good fit...