-
Notifications
You must be signed in to change notification settings - Fork 51
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
fix no-std support #145
fix no-std support #145
Conversation
Codecov Report
@@ Coverage Diff @@
## main #145 +/- ##
=======================================
Coverage 96.01% 96.01%
=======================================
Files 15 15
Lines 4065 4065
=======================================
Hits 3903 3903
Misses 162 162
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
190b894
to
575f25c
Compare
dc8911a
to
7112ecc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I think the different performance characteristics of the BTreeMap
vs HashMap
are probably not significant enough in this use-case to merit trying to do more complicated abstraction to allow switching between the two based on features.
Running cargo bench
on tip of main
and on this branch shows that the BTreeMap
-based owned representation is faster for the initial parsing of a chonky CRL and only a bit slower for lookups. I think it's a reasonable trade-off.
HashMap:
test bench_parse_owned_crl_large ... bench: 517,452,454 ns/iter (+/- 98,860,890)
test bench_parse_owned_crl_medium ... bench: 176,374,780 ns/iter (+/- 32,024,576)
test bench_parse_owned_crl_small ... bench: 186,129 ns/iter (+/- 7,450)
test bench_search_owned_crl_large ... bench: 9 ns/iter (+/- 3)
test bench_search_owned_crl_medium ... bench: 9 ns/iter (+/- 3)
test bench_search_owned_crl_small ... bench: 10 ns/iter (+/- 1)
BTreeMap:
test bench_parse_owned_crl_large ... bench: 296,112,924 ns/iter (+/- 37,342,479)
test bench_parse_owned_crl_medium ... bench: 89,594,215 ns/iter (+/- 13,807,766)
test bench_parse_owned_crl_small ... bench: 175,042 ns/iter (+/- 19,676)
test bench_search_owned_crl_large ... bench: 63 ns/iter (+/- 16)
test bench_search_owned_crl_medium ... bench: 69 ns/iter (+/- 13)
test bench_search_owned_crl_small ... bench: 50 ns/iter (+/- 2)
To be fair, lookups are like 7x slower with a |
@ctz Thoughts? Ready for merge queue? |
I'm going to bypass the merge queue for this one while the dependency fetching flake is happening. It keeps getting bounced from the queue 🎰 |
this PR
alloc
feature not pull in the optionalring
dependencycargo build --no-default-features --features alloc
fixes #143
fixes #144