Skip to content

Conversation

john-michaelburke
Copy link
Collaborator

@john-michaelburke john-michaelburke commented Feb 23, 2021

  • Moved process_messages out of server.rs.
  • Added criterion benchmark to run many iterations on small file (1 second). With the hopes to eventually add more specific tests including specific message types.
  • Added a scalable python script to enforce the benchmark regressions. Criterion has uses baseline files to do it's validation but we would effectively need one baseline file per benchmark per operating system.

NOTE: It seems this benchmark does not ever complete in Windows locally or in github actions. I suspect there is a difference in how Windows and Linux are handling the spawned thread in the benchmark. May need some other rustaceans to point out what I could be doing wrong.
I created a ticket here to address the Windows issue. https://swift-nav.atlassian.net/browse/CPP-67

Also since the benchmarks seem to be longer running I figure theyd be best parallelized with the Build Checks stage then Installers requires both stages to complete.

@john-michaelburke john-michaelburke force-pushed the johnmichael-burke/backend_benchmark branch 10 times, most recently from 07c484a to 1b1668c Compare February 24, 2021 01:51
@john-michaelburke john-michaelburke marked this pull request as ready for review February 24, 2021 02:09
@john-michaelburke john-michaelburke force-pushed the johnmichael-burke/backend_benchmark branch from 1b1668c to 55f50d4 Compare February 24, 2021 22:44
@john-michaelburke john-michaelburke force-pushed the johnmichael-burke/backend_benchmark branch from 55f50d4 to a1f1e3f Compare February 24, 2021 23:01
@john-michaelburke
Copy link
Collaborator Author

@silverjam Took the cake on this PR. Everything looks good to me though!

group.measurement_time(time::Duration::from_millis(BENCHMARK_TIME_LIMIT));
group.sample_size(BENCHMARK_SAMPLE_SIZE);
group.bench_function("RPM_failure", |b| {
b.iter(|| run_process_messages_failure(file_in_name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, I think we can easily refactor this to just use a "if (failure) { blah }"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. It seems you can just add another "bench_function" and they can be contained within the same group.

Copy link
Contributor

@silverjam silverjam left a comment

Choose a reason for hiding this comment

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

code quality checks need some ❤️ but otherwise lgtm

@john-michaelburke john-michaelburke force-pushed the johnmichael-burke/backend_benchmark branch from 56286ae to 6f4356f Compare February 26, 2021 00:03
messages: impl Iterator<Item = sbp::Result<SBP>>,
client_send_clone: mpsc::Sender<Vec<u8>>,
) {
let mut hpoints: Vec<(f64, OrderedFloat<f64>)> = vec![];
Copy link
Contributor

Choose a reason for hiding this comment

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

A type alias type Point = (f64, OrderedFloat<f64>) might be nice here

let mut hpoints: Vec<(f64, OrderedFloat<f64>)> = vec![];
let mut vpoints: Vec<(f64, OrderedFloat<f64>)> = vec![];
let mut sat_headers: Vec<u8> = vec![];
let mut sats: Vec<Vec<(f64, OrderedFloat<f64>)>> = vec![];
Copy link
Contributor

Choose a reason for hiding this comment

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

sats could probably be HashMap<u8, Vec<Point>>

@john-michaelburke john-michaelburke merged commit a019a2c into main Feb 26, 2021
@john-michaelburke john-michaelburke deleted the johnmichael-burke/backend_benchmark branch February 26, 2021 00:57
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.

3 participants