Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 35% faster decompression with less boundary check #41

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

XiangpengHao
Copy link
Contributor

Hi SpiralDB devs! First of all, thank you for implementing the great FSST library!

While profiling a decompression workload, I noticed we have some unintended boundary checks that could be removed to improve decompression throughput by 35%, as shown in the decompress benchmark below.

Removing those boundary checks requires more unsafe code, I've mannually annotated their correctness, but please double check! cc @alamb who might be interested

cargo bench --bench micro decompress

   Compiling fsst-rs v0.4.2 (/home/hao/coding/fsst)
    Finished `bench` profile [optimized] target(s) in 0.86s
     Running benches/micro.rs (target/release/deps/micro-bf2c70cedbbe0467)
Gnuplot not found, using plotters backend
cf=8/decompress         time:   [47.687 µs 47.725 µs 47.768 µs]
                        thrpt:  [20.444 GiB/s 20.462 GiB/s 20.478 GiB/s]
                 change:
                        time:   [-26.409% -26.311% -26.208%] (p = 0.00 < 0.05)
                        thrpt:  [+35.517% +35.705% +35.887%]
                        Performance has improved.
Found 11 outliers among 100 measurements (11.00%)
  6 (6.00%) high mild
  5 (5.00%) high severe

@CLAassistant
Copy link

CLAassistant commented Oct 3, 2024

CLA assistant check
All committers have signed the CLA.

@XiangpengHao XiangpengHao changed the title 35% better throughput with less boundary check 35% faster decompression with less boundary check Oct 3, 2024
@alamb
Copy link

alamb commented Oct 3, 2024

FYI @a10y

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@a10y
Copy link
Contributor

a10y commented Oct 3, 2024

Thanks @XiangpengHao ! Just two small comments (perhaps I'm being overly paranoid about add vs byte_add but just prefer to be explicit).

XiangpengHao and others added 2 commits October 3, 2024 09:26
Co-authored-by: Andrew Duffy <[email protected]>
Co-authored-by: Andrew Duffy <[email protected]>
@XiangpengHao
Copy link
Contributor Author

Thanks for the review! I've updated the code and rerun the benchmark and see no perf difference (just to be sure).

@lwwmanning lwwmanning changed the title 35% faster decompression with less boundary check feat: 35% faster decompression with less boundary check Oct 3, 2024
@lwwmanning lwwmanning enabled auto-merge (squash) October 3, 2024 16:20
@lwwmanning lwwmanning merged commit 4302023 into spiraldb:develop Oct 3, 2024
3 checks passed
@github-actions github-actions bot mentioned this pull request Sep 30, 2024
lwwmanning pushed a commit that referenced this pull request Oct 3, 2024
## 🤖 New release
* `fsst-rs`: 0.4.2 -> 0.4.3 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.4.3](v0.4.2...v0.4.3) -
2024-10-03

### Added

- 35% faster decompression with less boundary check
([#41](#41))

### Other

- *(deps)* update rust crate curl to v0.4.47
([#40](#40))
- *(deps)* update mozilla-actions/sccache-action action to v0.0.6
([#38](#38))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@a10y
Copy link
Contributor

a10y commented Oct 3, 2024

@XiangpengHao this change is now published in 0.4.3 https://crates.io/crates/fsst-rs/0.4.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants