Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate more compile benchmark metadata into the dashboard #1626

Merged
merged 9 commits into from
Jul 26, 2023

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Jun 29, 2023

This PR adds a new required field to perf-config.json, artifact. It has to be either library or binary, it specifies which target does the benchmark build. There's now a unit test that checks that all perf-config.json contain this attribute and that they be correctly parsed. Before this would only be found out during actual benchmarking or when displaying the compare page.

Furthermore, information about compile-time benchmarks that is useful (e.g. did the benchmark override LTO/CGU/debug information? does it build a binary or a library?) is now propagated into the compare page dashboard.

The information is extracted from the compile benchmark directory (and its perf-config.json and Cargo.toml files) via a build script of the site package. It is then sent to the compage page via the API and displayed as a simple tooltip over each benchmark name in the comparison table. The build script takes ~0.05s to execute, and is cached unless something in collector/compile-benchmarks changes (it seems that the detection whether something has changed is quite fast and doesn't slow down site rebuilds).

In addition, the filters can now hide/show all binaries/all libraries. This filtering is implemented using the newly added metadata.

perf

Fixes: #1619

@Kobzol Kobzol requested a review from lqd June 30, 2023 20:23
@Kobzol Kobzol force-pushed the benchmark-info-dashboard branch 2 times, most recently from 01cc3c8 to cd315d9 Compare July 17, 2023 07:24
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

This LGTM but I'm not familiar with:

  • some parts of the collector/api
  • using serde in real life, and the intricacies of missing data, backwards compatibility in format, deserialization errors, etc

so maybe another quick look from someone more familiar with these could be worthwhile

site/src/benchmark_metadata/mod.rs Outdated Show resolved Hide resolved
@Kobzol

This comment was marked as outdated.

@Kobzol

This comment was marked as outdated.

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 17, 2023

The frontend side was reviewed by @lqd, also requesting review from @Mark-Simulacrum about the config changes and the propagation of data into site.

@nnethercote
Copy link
Contributor

Do these instructions need updating?

@Kobzol
Copy link
Contributor Author

Kobzol commented Jul 18, 2023

Good point! I updated it, along with the glossary.

@Kobzol Kobzol enabled auto-merge July 26, 2023 06:43
@Kobzol Kobzol merged commit 531080d into rust-lang:master Jul 26, 2023
@Kobzol Kobzol deleted the benchmark-info-dashboard branch July 26, 2023 06:58
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.

Add more information to benchmarks on the compare page
4 participants