Replace standard library HashMap / HashSet with ahash #53
Replace standard library HashMap / HashSet with ahash #53Tristramg merged 3 commits intorust-transit:mainfrom
Conversation
|
Hi, thank you for your pull request. That’s an interesting simple and promising performance improvement for such a small change. I would be in favor, but I didn’t notice an improvement; I simply ran the the binary on the pbf of the netherlands. Could you share a bit more the benchmark you did (the code, the dataset)? |
|
Hi, i used function similar to "main" with fixed args. Datasets - Yaroslavl and Barcelona roads (pre-filtered for highway only with osmium). Also added std::time::Instant:now() ... elapsed() check in main, and ran on full denmark pbf. Time dropped from ~37 seconds to ~30. use criterion::{criterion_group, criterion_main, Criterion};
fn benchmark_main() {
let mut reader = osm4routing::Reader::new();
let file = "/home/chingiz/Rust/osm/roads_yar.pbf";
match reader.read(file) {
Ok((nodes, edges)) => osm4routing::writers::csv(nodes, edges, "nodes.csv", "edges.csv"),
Err(error) => println!("Error: {error}"),
}
}
fn criterion_benchmark(c: &mut Criterion) {
c.bench_function("benchmark_main", |b| b.iter(benchmark_main));
}
criterion_group!(benches, criterion_benchmark);
criterion_main!(benches); |
benchmark with unfiltered Denmark |
|
Thank you! |
|
@Tristramg can you approve the workflow, please |
|
@Tristramg Ready to merge, i have no push access |
The std::collections::HashMap uses a DOS-resistant but relatively slow hashing algorithm. In the case of osm4routing, HashMap is heavily used while reading .pbf files, even though cryptographic security is not required.
So why don't we replace it with much faster and lighter ahash instead?
my small criterion benchmark with osm.pbf of 500.000 people city highways shows around 18% perf improvement:
The change does not seem to break any public API. However, since it involves a type change, it may require a major version bump (0.8 ?)