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

bugfix, comment fix, force compile fails for big-endian #5

Merged
merged 3 commits into from
Aug 15, 2024

Conversation

a10y
Copy link
Contributor

@a10y a10y commented Aug 15, 2024

Also add tests with chinese characters

@@ -48,40 +45,5 @@ fn bench_fsst(c: &mut Criterion) {
});
}

fn bench_lz4(c: &mut Criterion) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these were kinda useless

@@ -9,6 +11,8 @@ that they should declare the causes which impel them to the separation."#;

static DECLARATION: &str = include_str!("./fixtures/declaration.txt");

static ART_OF_WAR: &str = include_str!("./fixtures/art_of_war.txt");
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 ⚔️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

let symbol2 = &self.symbols[code2 as usize];
// If either symbol is zero-length, or if merging would yield a symbol of
// length greater than 8, skip.
if symbol1.len() + symbol2.len() >= 8 || symbol1.is_empty() || symbol2.is_empty() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

symbols can never be empty, the last PR changed that

@@ -103,19 +103,19 @@ impl SymbolTable {
fn optimize(&self, counters: Counter) -> Self {
let mut res = SymbolTable::default();
let mut pqueue = BinaryHeap::new();
for code1 in 0..511 {
for code1 in 0u16..(256u16 + self.n_symbols as u16) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing the loop bound here and below gave us a nice little speedup

image

@a10y a10y merged commit f2279a0 into develop Aug 15, 2024
1 check passed
@a10y a10y deleted the aduffy/followup branch August 15, 2024 16:28
@github-actions github-actions bot mentioned this pull request Aug 15, 2024
a10y pushed a commit that referenced this pull request Aug 15, 2024
## 🤖 New release
* `fsst-rs`: 0.0.1

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

<blockquote>

## [0.0.1](https://github.com/spiraldb/fsst/releases/tag/v0.0.1) -
2024-08-15

### Fixed
- fix doc link

### Other
- turn on release-plz
- add fuzzer, fix bug ([#7](#7))
- logo ([#6](#6))
- bugfix, comment fix, force compile fails for big-endian
([#5](#5))
- Configure Renovate ([#1](#1))
- Get compress performance to match paper algorithm 4
([#3](#3))
- docs
- cleanup
- words
- README
- disable release action for now
- deny(missing_docs), 512 -> 511
- add toolchain
- add actions files
- implementation v0
- initial impl
- Initial commit
</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>
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