Keats / jsonwebtoken

JWT lib in rust
MIT License
1.69k stars 271 forks source link

increasing performance over validation of audiences #176

Closed sabify closed 3 years ago

sabify commented 3 years ago

Hello,

We don't need to iterate over all possible hits in two sets by counting them, we just need the first hit to validate.

sabify commented 3 years ago

Here is the difference:

It's about 2.5x performance gain.

bench_hashmap_count     time:   [43.185 ns 43.226 ns 43.271 ns]
Found 9 outliers among 100 measurements (9.00%)
  6 (6.00%) high mild
  3 (3.00%) high severe

bench_hashmap_next      time:   [16.125 ns 16.128 ns 16.133 ns]
Found 13 outliers among 100 measurements (13.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  8 (8.00%) high severe

Code:

fn bench_hashmap(c: &mut Criterion) {
    let auds1: std::collections::HashSet<String> =
        ["1".to_string(), "2".to_string(), "3".to_string()].iter().cloned().collect();
    let auds2: std::collections::HashSet<String> =
        ["1".to_string(), "2".to_string(), "3".to_string()].iter().cloned().collect();
    c.bench_function("bench_hashmap_count", |b| b.iter(|| auds1.intersection(&auds2).count()));
    c.bench_function("bench_hashmap_next", |b| {
        b.iter(|| auds1.intersection(&auds2).next().is_none())
    });
}
Keats commented 3 years ago

I meant across the whole decoding process, I'm guessing it's negligeable

sabify commented 3 years ago

But affect the performance a lot in its place! specially on validating multiple validation audiences over multiple claim audiences. I still don't get it you've said I would go for the clearer approach with count... Why .next().is_none() is not clear approach but .count() == 0 is?

Keats commented 3 years ago

Why .next().is_none() is not clear approach but .count() == 0 is?

Probably just a personal thing. I'm happy to merge it if it's not a common viewpoint.

But affect the performance a lot in its place! specially on validating multiple validation audiences over multiple claim audiences.

Over the whole decoding, it's just a rounding error. 20ns out of 2800ns for a whole decoding on my machine