Skip to content

Create bench_cmp command #1955

New issue

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

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

Already on GitHub? # to your account

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.

@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.

@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.

@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
@s7tya s7tya force-pushed the add-bench-cmp-command branch from 0e04b34 to 383b3c2 Compare August 6, 2024 07:59
@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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants