feat: add snappy binding - #22
Conversation
Summary of ChangesHello @wemeetagain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates Snappy compression and decompression capabilities into the project. It achieves this by adding a Zig FFI binding for the Snappy library, providing a TypeScript wrapper for these functions, and establishing a robust set of tests and benchmarks to validate its functionality and performance. The changes span across build configurations, dependency management, FFI definitions, and new source files for the Snappy implementation and its associated tests and benchmarks. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces Snappy compression bindings, including the Zig implementation, TypeScript wrappers, tests, and benchmarks. The overall implementation is well-structured. However, I've identified a critical copy-paste error in the TypeScript wrapper that results in incorrect error handling by using an error pointer from the LevelDB binding instead of the one for Snappy. This needs to be corrected to ensure proper error reporting.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
on m2 mac without -O2 flag:
bench/snappy.bench.ts
snappy
✔ compress 693481.3 ops/s 1.442000 us/op - 246689 runs 0.404 s
✔ compress (other) 1663894 ops/s 601.0000 ns/op - 3577853 runs 2.97 s
✔ uncompress 1406470 ops/s 711.0000 ns/op - 1562453 runs 1.41 s
✔ uncompress (other) 3546099 ops/s 282.0000 ns/op - 1535725 runs 0.707 s
with -O2 flag:
bench/snappy.bench.ts
snappy
✔ compress 1862197 ops/s 537.0000 ns/op - 1969556 runs 1.53 s
✔ compress (other) 1686341 ops/s 593.0000 ns/op - 781233 runs 0.609 s
✔ uncompress 3546099 ops/s 282.0000 ns/op - 992164 runs 0.506 s
✔ uncompress (other) 3424658 ops/s 292.0000 ns/op - 2207266 runs 1.11 s
There was a problem hiding this comment.
with ChainSafe/snappy.zig#2 the performance regression should be addressed, though there is probably still more to do there, there are some platform specific optimization flags in the CMakeLists.txt that we're not taking advantage of.
-O2