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

Create bench_cmp command #1955

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

s7tya
Copy link
Contributor

@s7tya s7tya commented Jul 29, 2024

I've implemented a small example command for comparing two artifacts. Currently, it only displays the range, mean, and count across all series. This is the first step towards #1734. I'm not sure if the way I'm collecting data is correct or not.

$ cargo +nightly run --release --bin collector bench_cmp a b

+--------+-------+------------------+--------+
| name   | count | range            | mean   |
+--------+-------+------------------+--------+
| ❌     | 43    | [-0.50%, -0.20%] | -0.28% |
+--------+-------+------------------+--------+
| ✅     | 68    | [+0.20%, +0.73%] | +0.32% |
+--------+-------+------------------+--------+
| ✅, ❌ | 111   | [-0.50%, +0.73%] | +0.09% |
+--------+-------+------------------+--------+

@s7tya s7tya force-pushed the add-bench-cmp-command branch 4 times, most recently from 72fcfc0 to 671a368 Compare July 29, 2024 08:32
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! :) I would like to avoid one big PR that will implement some super-duper UI at once, so let's first land the basic CLI interface, and just output a single useful piece of information (which compiler is faster on instruction counts on average, and by how much). Then you can improve the UI incrementally piece by piece.

collector/src/bin/collector.rs Outdated Show resolved Hide resolved
collector/src/bin/collector.rs Outdated Show resolved Hide resolved
collector/src/bin/collector.rs Show resolved Hide resolved
@Kobzol
Copy link
Contributor

Kobzol commented Jul 29, 2024

To clarify: even though I suggested just to output a simple text line for simplicity, you can keep the table that you have right now, if you want. Although I suppose that soon it will be replaced by a more complex UI anyway.

collector/src/lib.rs Outdated Show resolved Hide resolved
@s7tya s7tya force-pushed the add-bench-cmp-command branch 2 times, most recently from d20abeb to a4a51f4 Compare July 29, 2024 09:59
@s7tya s7tya changed the title Create bench_local_diff command Create bench_cmp command Jul 29, 2024
@s7tya
Copy link
Contributor Author

s7tya commented Jul 29, 2024

Thanks! I've fixed the requested changes.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

It's really cool to be able to just run a CLI command and instantly see the difference :) This is gonna be very useful. Left a few more comments.

collector/src/bin/collector.rs Outdated Show resolved Hide resolved
collector/src/bin/collector.rs Outdated Show resolved Hide resolved
collector/src/compare.rs Outdated Show resolved Hide resolved
collector/src/compare.rs Outdated Show resolved Hide resolved
collector/src/compare.rs Outdated Show resolved Hide resolved
collector/src/compare.rs Outdated Show resolved Hide resolved
collector/src/compare.rs Show resolved Hide resolved
@s7tya s7tya force-pushed the add-bench-cmp-command branch 3 times, most recently from 10a6472 to 6bc5839 Compare August 5, 2024 23:36
@s7tya s7tya marked this pull request as draft August 6, 2024 07:13
@s7tya

This comment was marked as outdated.

@s7tya s7tya force-pushed the add-bench-cmp-command branch 2 times, most recently from 85f7578 to 0e04b34 Compare August 6, 2024 07:55
@s7tya s7tya marked this pull request as ready for review August 6, 2024 07:55
collector/src/bin/collector.rs Outdated Show resolved Hide resolved
site/src/comparison.rs Outdated Show resolved Hide resolved
@s7tya s7tya force-pushed the add-bench-cmp-command branch 2 times, most recently from 3dc4ea8 to 9189a3a Compare August 6, 2024 08:16
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Awesome. Tried it locally and it works great. Thank you!

@Kobzol Kobzol enabled auto-merge August 6, 2024 08:19
@Kobzol Kobzol merged commit 239b046 into rust-lang:master Aug 6, 2024
11 checks passed
@s7tya s7tya deleted the add-bench-cmp-command branch August 9, 2024 07:17
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.

2 participants