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

298 fix build of version 0.9.5 #299

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Conversation

archeoss
Copy link
Contributor

@archeoss archeoss commented Nov 4, 2023

Resolves #298

Some additional info:

  • Benchmark result with ahash crate:
----------------RESULTS-----------------
     reports total:            1112
     reports collected:        1112
     test duration:           1.086
     written total, MB:     100.089
     rate, MB/s:             92.145
     rate, recs/s          1023.741
     avg latency:           9.058ms
  • Benchmark result in current version:
----------------RESULTS-----------------
     reports total:            1112
     reports collected:        1112
     test duration:           1.092
     written total, MB:     100.089
     rate, MB/s:             91.615
     rate, recs/s          1017.855
     avg latency:           9.105ms
  • Copied version of aHash: 0.7.4(with added one typo fix as discussed inhere)

  • About this test with hash matches:

    • Invoking cargo test without additional flags passes the test
    • Invoking RUSTFLAGS="-C target-feature=+aes,+ssse3" cargo test --target=x86_64-unknown-linux-musl fails the test (hashes don't match)
    • This behaviour is consistent with both copied version of ahash and with imported one from github using Cargo github import (faulty test or expected behaviour?)

@archeoss archeoss linked an issue Nov 5, 2023 that may be closed by this pull request
Cargo.toml Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
src/filter/ahash/mod.rs Outdated Show resolved Hide resolved
@archeoss
Copy link
Contributor Author

Some additional info:

sample tests:

hasher1.write(&(0..10).collect::<Vec<u8>>());
hasher2.write(&(245..255).collect::<Vec<u8>>());
hasher3.write(&(63..73).collect::<Vec<u8>>());
hasher4.write(&(101..111).collect::<Vec<u8>>());

Output without flags (different results for aHash v0.8.6 due to different keys):

ver. 1 2 3 4
aHash v.0.7.4 3604729491498336444 4698010058046694585 7892047681755360091 15822444892006722439
aHash v.0.7.6 3604729491498336444 4698010058046694585 7892047681755360091 15822444892006722439
aHash v.0.8.6 7715258586443628156 10697497720697129369 4211484261640213197 9763537285513611305

Output with RUSTFLAGS="-C target-feature=+aes,+ssse3" flags:

ver. 1 2 3 4
aHash v.0.7.4 16011136255442261133 2605215798843651484 6547545895333015968 6902404632462942265
aHash v.0.7.6 16011136255442261133 2605215798843651484 6547545895333015968 6902404632462942265
aHash v.0.8.6 15475331549103311308 7344361408891437224 13367513670482924484 4821293288195121582

Output with RUSTFLAGS="-C target-feature=+ssse3" flags:

ver. 1 2 3 4
aHash v.0.7.4 3604729491498336444 4698010058046694585 7892047681755360091 15822444892006722439
aHash v.0.7.6 3604729491498336444 4698010058046694585 7892047681755360091 15822444892006722439
aHash v.0.8.6 7715258586443628156 10697497720697129369 4211484261640213197 9763537285513611305

Conclusion: aHash results dependent on aes flag

Cargo.toml Outdated Show resolved Hide resolved
@ikopylov
Copy link
Member

Please, fix a warning in src/blob/index/mod.rs: unused import: 'crate::prelude::*'

src/lib.rs Outdated Show resolved Hide resolved
src/filter/mod.rs Outdated Show resolved Hide resolved
tests/tests.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@ikopylov ikopylov left a comment

Choose a reason for hiding this comment

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

LGTM

@ikopylov ikopylov merged commit 4637dbe into release_v0.9.6 Nov 20, 2023
3 checks passed
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.

Fix build of version 0.9.5
2 participants