BrianPugh/tamp

ESP32 Optimized Compressor

BrianPugh opened this issue · 7 comments

Hey @BitsForPeople!

I was giving your esp32-optimized tamp implementation a try, as I would love to integrate those optimizations into this repo with the following goals:

  1. Publishing a component package on the esp-component-registry.
  2. Improve the performance of the micropython native module for xtensawin targets.

I made a quick and dirty demo compressing the first 100,000 bytes of the enwik8 dataset. On an esp32, without the optimizations enabled, it compresses the data down to the expected 51,637 bytes in 5.76 seconds. With optimizations enabled, it seems to only compress 17 output bytes and then it thinks compression is done. Any ideas on why this may be? Thanks!

Thanks for testing and reporting. I'll try to reproduce using your demo and see what's wrong.

Seems there was an issue with the build system mixing the ESP32 and non-ESP32 header file variants. Should be fixed now (ccae6f1).

It works now! thank you so much for the quick fix! On my esp32 it's 2.5x faster than the original implementation!

Are you ok with me merging in your code? I'll probably re-organize the files a little bit, but it'll keep your headers and I'll also add an acknowledgement section to the README.md

Another question: are there speed-related reasons for this function signature change here?. Or was it simplfy for convenience? I kept the return value as void incase I ever need to return an error code from that function (and so that its consistent with all the other tamp functions). In the code I merge in I'd like to not change the signature.

Are you ok with me merging in your code?

Of course, please go ahead :)

Another question: are there speed-related reasons for this function signature change here?. Or was it simplfy for convenience?

I think it was more for convenience, not sure if it made any measurable performance improvement. Should be good to revert back to the original signature.

This is now a part of v1.5.0!