benches: CRL parsing/searching benchmarks. by cpu · Pull Request #108 · rustls/webpki

cpu · 2023-07-03T13:52:47Z

This branch adds two kinds of CRL benchmark tests:

Parsing benchmarks, testing how long it takes to use BorrowedCertRevocationList::from_der, as well as how long it takes to do the same converting to the OwnedCertRevocationList representation that allocates.
Searching benchmarks, testing how long it takes to search an already parsed CRL for a serial number that doesn't appear in the CRL. This is done for both the borrowed and owned representations.

In both cases we add benchmarks that use three sizes of CRLs:

a small CRL (2,000 serials, ~72kb)
a medium CRL (600,000 serials, ~22mb)
a large CRL (1,500,000 serials, ~50mb)

Since large test files aren't suitable for inclusion in git, these CRLs are generated on first benchmark run using rcgen. We use a Cargo patch to use the master branch of this dev-dependency for CRL support.

Initial results from my laptop:

Running benches/benchmark.rs (target/release/deps/benchmarks-b6e36ee32c803b65)

running 12 tests
test bench_parse_borrowed_crl_large   ... bench:          49 ns/iter (+/- 12)
test bench_parse_borrowed_crl_medium  ... bench:          51 ns/iter (+/- 4)
test bench_parse_borrowed_crl_small   ... bench:          53 ns/iter (+/- 1)
test bench_parse_owned_crl_large      ... bench: 502,094,511 ns/iter (+/- 106,915,574)
test bench_parse_owned_crl_medium     ... bench: 183,980,410 ns/iter (+/- 10,231,635)
test bench_parse_owned_crl_small      ... bench:     188,714 ns/iter (+/- 9,487)
test bench_search_borrowed_crl_large  ... bench:  42,466,919 ns/iter (+/- 4,490,349)
test bench_search_borrowed_crl_medium ... bench:  16,940,885 ns/iter (+/- 823,770)
test bench_search_borrowed_crl_small  ... bench:      53,643 ns/iter (+/- 7,362)
test bench_search_owned_crl_large     ... bench:          10 ns/iter (+/- 4)
test bench_search_owned_crl_medium    ... bench:           9 ns/iter (+/- 2)
test bench_search_owned_crl_small     ... bench:          10 ns/iter (+/- 1)

test result: ok. 0 passed; 0 failed; 0 ignored; 12 measured

Laptop specs:

rustc 1.69.0
NixOS 22.11
12th Gen Intel(R) Core(TM) i7-1280P
32GB DDR4-3200
WD_BLACK SN850 NVMe

codecov · 2023-07-03T13:54:57Z

Codecov Report

Merging #108 (7e7157b) into main (d9b4338) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #108   +/-   ##
=======================================
  Coverage   95.36%   95.36%           
=======================================
  Files          15       15           
  Lines        3346     3346           
=======================================
  Hits         3191     3191           
  Misses        155      155

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

cpu · 2023-07-03T13:56:09Z

ci / msrv (pull_request) Failing after 1m

Hmm, I thought we fixed this by narrowing the check to the primary library crate.

ci / deny (pull_request) Failing after 1m

If we want to merge this branch before rcgen cuts a tag we'll need to relax this check to allow the git patch for the dev dependency.

cpu · 2023-07-03T14:27:47Z

Hmm, I thought we fixed this by narrowing the check to the primary library crate.

Dropping once_cell dev dep a version seems to do the trick so I'll go with that for now.

If we want to merge this branch before rcgen cuts a tag we'll need to relax this check to allow the git patch for the dev dependency.

Added a cargo deny exception for the rcgen repo with a note to remove in the future.

ctz · 2023-07-03T14:30:00Z

Initial results from my laptop:

Running benches/benchmark.rs (target/release/deps/benchmarks-c7a846b8f24c0c08)

running 6 tests
test bench_parse_crl_large   ... bench:          47 ns/iter (+/- 5)
test bench_parse_crl_medium  ... bench:          53 ns/iter (+/- 3)
test bench_parse_crl_small   ... bench:          54 ns/iter (+/- 5)
test bench_search_crl_large  ... bench:  35,309,161 ns/iter (+/- 1,290,180)
test bench_search_crl_medium ... bench:  14,219,886 ns/iter (+/- 540,508)
test bench_search_crl_small  ... bench:      46,616 ns/iter (+/- 972)

test result: ok. 0 passed; 0 failed; 0 ignored; 6 measured

Are these results from before the hash-table change? I get these:

test bench_parse_crl_large   ... bench:          89 ns/iter (+/- 8)
test bench_parse_crl_medium  ... bench:          91 ns/iter (+/- 2)
test bench_parse_crl_small   ... bench:          91 ns/iter (+/- 1)
test bench_search_crl_large  ... bench:           8 ns/iter (+/- 1)
test bench_search_crl_medium ... bench:           9 ns/iter (+/- 2)
test bench_search_crl_small  ... bench:           8 ns/iter (+/- 0)

cpu · 2023-07-03T14:33:06Z

Are these results from before the hash-table change? I get these:

I updated the comment when I rebased this branch on top of the reworked representation branch a few days ago.

I can reproduce what you're seeing locally when I re-run the benchmarks from scratch today. Very odd because the benchmarks are using the BorrowedCertRevocationList representation that uses the linear search.

cpu · 2023-07-03T14:44:09Z

Flipping this to a draft while I dig into what's going on with the search benchmark results. I thought maybe something was getting optimized out but adding a blackbox hint hasn't fixed the problem. Will investigate

cpu · 2023-07-03T14:54:26Z

Ahhh! I think I figured it out. I had changed how I was populating the serial numbers in the revoked certs of the generated test CRLs.

When I re-ran the benchmarks the other day to get new #'s I had CRLs already generated that used the previous approach and so the benchmarks ran against those. You generated new CRLs, and when I re-tested today I did the same. Those CRLs have some negative serial numbers and the linear search bails on the first invalid serial number, ending the search prematurely.

I'll fix the serial number generation and also update the benchmark methods so that they ensure no errors were encountered in the search.

Thanks for catching that.

ctz · 2023-07-03T14:54:53Z

adding an .unwrap() to the find_serial return I don't think invalidates the benchmark, but does show up:

test bench_search_crl_large  ... thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: InvalidSerialNumber', benches/benchmark.rs:139:54
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

edit: snap!

cpu · 2023-07-03T15:05:13Z

I'll fix the serial number generation and also update the benchmark methods so that they ensure no errors were encountered in the search.

Done and the benchmark times are back to what I was seeing before:

[daniel@blanc:~/Code/Rust/webpki]$ git rev-parse HEAD
a96b124b70de62a3205ef526b1042eb021a36fe2

[daniel@blanc:~/Code/Rust/webpki]$ rm benches/*.der

[daniel@blanc:~/Code/Rust/webpki]$ cargo bench
   Compiling rustls-webpki v0.100.1 (/home/daniel/Code/Rust/webpki)
    Finished bench [optimized] target(s) in 4.24s
     Running unittests src/lib.rs (target/release/deps/webpki-65f9e31d579c4dd5)
     
<snipped>

     Running benches/benchmark.rs (target/release/deps/benchmarks-de669096ef76485d)

running 6 tests
test bench_parse_crl_large   ... bench:          46 ns/iter (+/- 17)
test bench_parse_crl_medium  ... bench:          50 ns/iter (+/- 2)
test bench_parse_crl_small   ... bench:          50 ns/iter (+/- 4)
test bench_search_crl_large  ... bench:  35,463,428 ns/iter (+/- 2,074,885)
test bench_search_crl_medium ... bench:  14,089,006 ns/iter (+/- 796,132)
test bench_search_crl_small  ... bench:      45,447 ns/iter (+/- 1,810)

test result: ok. 0 passed; 0 failed; 0 ignored; 6 measured

cpu · 2023-07-04T16:25:18Z

@ctz Does the current revision benchmark correctly for you locally? You will need to delete the benches/*.der files that were generated with the buggy revision beforehand to get back to a good state.

ctz · 2023-07-04T16:33:06Z

Yep, just retested with a96b124 and I got reasonable figures.

ctz · 2023-07-04T17:03:18Z

Worth extending these to show the difference with the owned types?

This commit adds two kinds of CRL benchmark tests: 1. Parsing benchmarks, testing how long it takes to use `BorrowedCertRevocationList::from_der`, as well as how long it takes to do the same converting to the `OwnedCertRevocationList` representation that allocates. 2. Searching benchmarks, testing how long it takes to search an already parsed CRL for a serial number that doesn't appear in the CRL. This is done for both the borrowed and owned representations. In both cases we add benchmarks that use three sizes of CRLs: 1. a small CRL (2,000 serials, ~72kb) 2. a medium CRL (600,000 serials, ~22mb) 3. a large CRL (1,500,000 serials, ~50mb) Since large test files aren't suitable for inclusion in git, these CRLs are generated on first benchmark run using `rcgen`. We use a Cargo patch to use the master branch of this dev-dependency for CRL support. Initial results from my laptop: ``` Running benches/benchmark.rs (target/release/deps/benchmarks-b6e36ee32c803b65) running 12 tests test bench_parse_borrowed_crl_large ... bench: 49 ns/iter (+/- 12) test bench_parse_borrowed_crl_medium ... bench: 51 ns/iter (+/- 4) test bench_parse_borrowed_crl_small ... bench: 53 ns/iter (+/- 1) test bench_parse_owned_crl_large ... bench: 502,094,511 ns/iter (+/- 106,915,574) test bench_parse_owned_crl_medium ... bench: 183,980,410 ns/iter (+/- 10,231,635) test bench_parse_owned_crl_small ... bench: 188,714 ns/iter (+/- 9,487) test bench_search_borrowed_crl_large ... bench: 42,466,919 ns/iter (+/- 4,490,349) test bench_search_borrowed_crl_medium ... bench: 16,940,885 ns/iter (+/- 823,770) test bench_search_borrowed_crl_small ... bench: 53,643 ns/iter (+/- 7,362) test bench_search_owned_crl_large ... bench: 10 ns/iter (+/- 4) test bench_search_owned_crl_medium ... bench: 9 ns/iter (+/- 2) test bench_search_owned_crl_small ... bench: 10 ns/iter (+/- 1) test result: ok. 0 passed; 0 failed; 0 ignored; 12 measured ``` Laptop specs: * rustc 1.69.0 * NixOS 22.11 * 12th Gen Intel(R) Core(TM) i7-1280P * 32GB DDR4-3200 * WD_BLACK SN850 NVMe

cpu · 2023-07-04T17:33:43Z

Worth extending these to show the difference with the owned types?

Done 👍 Also updated the PR description with fresh results.

cpu self-assigned this Jul 3, 2023

cpu force-pushed the cpu-bench-crls branch 2 times, most recently from b8baf6c to 96d3ffa Compare July 3, 2023 14:20

cpu marked this pull request as draft July 3, 2023 14:43

cpu force-pushed the cpu-bench-crls branch from 96d3ffa to a96b124 Compare July 3, 2023 15:02

cpu marked this pull request as ready for review July 3, 2023 15:05

cpu force-pushed the cpu-bench-crls branch from a96b124 to 7e7157b Compare July 4, 2023 17:31

ctz approved these changes Jul 6, 2023

View reviewed changes

cpu merged commit 0b43fb8 into rustls:main Jul 6, 2023

cpu deleted the cpu-bench-crls branch July 6, 2023 14:23

Conversation

cpu commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cpu commented Jul 3, 2023

Uh oh!

cpu commented Jul 3, 2023

Uh oh!

ctz commented Jul 3, 2023

Uh oh!

cpu commented Jul 3, 2023

Uh oh!

cpu commented Jul 3, 2023

Uh oh!

cpu commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctz commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpu commented Jul 3, 2023

Uh oh!

cpu commented Jul 4, 2023

Uh oh!

ctz commented Jul 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctz commented Jul 4, 2023

Uh oh!

cpu commented Jul 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cpu commented Jul 3, 2023 •

edited

Loading

codecov bot commented Jul 3, 2023 •

edited

Loading

cpu commented Jul 3, 2023 •

edited

Loading

ctz commented Jul 3, 2023 •

edited

Loading

ctz commented Jul 4, 2023 •

edited

Loading