feat: Add optional lz4 compression support for arrays passed via base64 or binref encoding - #579
Conversation
|
@dionhaefner @nmheim Let me know if this is what you meant by testing compression in tesseract? |
|
That's a good start, thanks @angela-ko ! As next step, please add minimal, meaningful end-to-end tests that cover this functionality - which I expect are going to fail because I do see some issues with how the new lz4 dependency is added :) Once everything is passing end-to-end I'll have a closer look at the design choices here. |
|
And please outline your rationale for choosing lz4 specifically as part of the PR body. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
==========================================
- Coverage 78.15% 78.03% -0.12%
==========================================
Files 39 39
Lines 4632 4685 +53
Branches 754 770 +16
==========================================
+ Hits 3620 3656 +36
- Misses 709 720 +11
- Partials 303 309 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Benchmark Resultsℹ️ No baseline found — all benchmarks marked as new. Benchmarks use a no-op Tesseract to measure pure framework overhead.
Benchmark details
|
56294af to
17fb949
Compare
8310d77 to
dc6a43b
Compare
dionhaefner
left a comment
There was a problem hiding this comment.
Taking shape – let's get some clarity on high-level design decisions before diving into details.
…ssion sie if compression is set
af7df61 to
b9ed79a
Compare
base64 or binref encoding
dionhaefner
left a comment
There was a problem hiding this comment.
Thanks @angela-ko. Looking real good now, just a last few comments.
|
@angela-ko Thanks for the feature! Please have a look at the updated PR body - remember to only link to public resources next time. |
Relevant issue or PR
n/a
Description of changes
Adds optional lz4 compression for arrays serialized via
json+base64andjson+binrefencodings, opt-in through a newTESSERACT_COMPRESSIONconfig var (currentlylz4or unset). lz4 was chosen as the first option because it's a small, single dependency with fast round-trips. Further codecs can be added later without touching the encode/decode call sites.TESSERACT_COMPRESSIONonly controls how a Tesseract encodes its output. It has no effect on inputs: each array carries its own"compression"field, so a Tesseract decodes any valid input — compressed or not — regardless of config. This means you can pipe a compressed output straight into another Tesseract without configuring the receiver.Key details:
lz4is a new runtime dependency for all Tesseract builds.Arrays are compressed individually rather than compressing the whole buffer, which preserves offset-based random access into binref files. Because compressed length isn't derivable from shape/dtype, the binref buffer spec is extended from
<path>:<offset>to<path>:<offset>:<compressed_size>, and a"compression"field is added to the array data dict. The buffer regex and validation pattern are updated accordingly; readers requirecompressed_sizewhencompressionis set.TESSERACT_COMPRESSIONis a runtime config var, so throughtesseract runit must be forwarded into the container with-e(a shell-level prefix stays on the host):Testing done
CI. New unit tests cover base64 and binref compress/decompress round-trips, the extended buffer-spec parsing, and the "missing compressed_size" error path; end-to-end
tesseract runtests exerciseTESSERACT_COMPRESSION=lz4for both encodings.New benchmark, too.