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: make Compressor::train 2x faster with bitmap index #16

Merged
merged 6 commits into from
Aug 20, 2024

Conversation

a10y
Copy link
Contributor

@a10y a10y commented Aug 20, 2024

The slowest part of Compressor::train is the double-nested loops over codes.

Now compress_count when it records code pairs will also populate a bitmap index, where pairs_index[code1].set(code2) will indicate that code2 followed code1 in compressed output.

In the optimize loop, we can eliminate tight loop iterations by accessing pairse_index[code1].second_codes() which yields the value code2 values.

This results in a speedup from ~1ms -> 500micros for the training benchmark. We're sub-millisecond!

This also makes Miri somewhat palatable to run for all but test_large, so I've re-enabled it for CI (currently it runs in 2.5 minutes. Far cry from the < 30s build+test step but I guess it's for a good cause)

a10y added 2 commits August 20, 2024 16:44
The slowest part of Compressor::train is the double-nested loops
over codes.

Now compress_count when it records code pairs will also populate
a bitmap index, where `pairs_index[code1].set(code2)` will indicate
that code2 followed code1 in compressed output.

In the `optimize` loop, we can eliminate tight loop iterations by
accessing `pairse_index[code1].second_codes()` which yields the value
code2 values.

This results in a speedup from ~1ms -> 500micros.
Comment on lines -35 to -40
pub fn reset(&mut self) {
for idx in 0..COUNTS1_SIZE {
self.counts1[idx] = 0;
}
for idx in 0..COUNTS2_SIZE {
self.counts2[idx] = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was slower than just building a new Counter b/c of the vec![0] change made in the previous PR

@a10y a10y force-pushed the aduffy/train-speedup branch from 64beee7 to f74d185 Compare August 20, 2024 21:58
@a10y a10y merged commit d7e836c into develop Aug 20, 2024
3 checks passed
@a10y a10y deleted the aduffy/train-speedup branch August 20, 2024 22:04
@github-actions github-actions bot mentioned this pull request Aug 20, 2024
a10y pushed a commit that referenced this pull request Aug 20, 2024
## 🤖 New release
* `fsst-rs`: 0.2.0 -> 0.2.1

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

<blockquote>

## [0.2.1](v0.2.0...v0.2.1) -
2024-08-20

### Added
- make Compressor::train 2x faster with bitmap index
([#16](#16))
</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 a10y mentioned this pull request Aug 20, 2024
Comment on lines +61 to +63
if self.block == 0 {
return None;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be possible to skip this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! #18

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.

2 participants