Support benchmarks using wall-time mode #14
Conversation
ci-bench-runner/src/bencher_dev.rs
Outdated
| return None; | ||
| } | ||
| }; | ||
| fn append_results_as_bmf( |
There was a problem hiding this comment.
This might be more of a style thing but I feel like my inclination would be to try and avoid passing in mutable state by having this function do the conversion of HashMap<String, f64>, MetricKind to BMF and return JsonMetricsMap, leaving pushing the BMF into the JsonResultsMap to the callers. Is there a reason to avoid that I'm overlooking?
There was a problem hiding this comment.
True, looks like I fell into the trap of premature optimization here
| icount_scenarios_missing_in_baseline TEXT, | ||
| walltime_scenarios_missing_in_baseline TEXT |
There was a problem hiding this comment.
The changes in this file feel like a case where we should be using the migration support to apply a delta over the pre-existing 0_create.sql. I think with this approach we'd have to drop the production database and recreate it from scratch right?
There was a problem hiding this comment.
We don't have a production database yet, so I think we can get away with this. Or is there a different reason why this might be a problem after all?
By the way, SQLx will raise a fatal error if a migration was modified after it was applied (nice to know SQLx has our back and will prevent accidents even if we mess things up).
There was a problem hiding this comment.
We don't have a production database yet, so I think we can get away with this. Or is there a different reason why this might be a problem after all?
Nope, I think I just tricked myself into thinking there was an instance on the dedicated server already and so it made sense to treat this as a migration layered on top 😅
By the way, SQLx will raise a fatal error if a migration was modified after it was applied (nice to know SQLx has our back and will prevent accidents even if we mess things up).
Oh great 👍 That does help alleviate my concern here.
See this comment for example output (from a run on the bare-metal server). The noise is incredibly low! If such low level of noise is consistent over time, we might not even need the icount benchmarks 🤔. By contrast, the previous comment shows results from a run on my laptop and are pretty noisy.
Relevant changes (pretty intertwined, so all in a single commit):
scenario_kindinbench_resultstableBtw, I discovered a few more ansible issues, which I'll tackle in a separate PR. There is also an issue in the sorting of the diffs, unrelated to this PR, which I'll fix separately too.