Skip to content

Conversation

@camelid
Copy link
Member

@camelid camelid commented Mar 2, 2021

Both I and @jyn514 had no idea that @rust-timer queue exclude
existed because there's no documentation, or at least I wasn't able to
find any publicized documentation.

r? @Mark-Simulacrum
cc @jyn514

@camelid
Copy link
Member Author

camelid commented Mar 2, 2021

The page looks like this on my local machine:

image

EDIT: Updated screenshot based on latest changes.

<li><code>exclude=&lt;EXCLUDE&gt;</code> is similar to <code>include=</code>, but instead skips any benchmarks that have one of the items in the list of substrings as a substring of its name.</li>
<li><code>runs=&lt;RUNS&gt;</code> configures how many times the benchmark is run. <code>&lt;RUNS&gt;</code> is an integer.</li>
</ul>
<p><code>@rust-timer</code> has more commands than just <code>@rust-timer queue</code>, but the <code>queue</code> command is the most used.</p>
Copy link
Member

Choose a reason for hiding this comment

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

What other commands does it support?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's also build, make-pr-for, and update-branch-for, but I can't document them because I don't anything about them (except for a little bit about make-pr-for).

Copy link
Member

Choose a reason for hiding this comment

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

I think documenting build is worthwhile, the other two are buggy and we shouldn't document them for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could we save documenting build for a later PR? I know nothing about it so I can't document it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok! It's pretty much equivalent to queue, just takes an additional SHA argument (useful if e.g. bors flakes and forgets to post or so)

<ul>
<li><code>include=&lt;INCLUDE&gt;</code> only runs benchmarks with <code>&lt;INCLUDE&gt;</code> in their title. For example, if you just want to run rustdoc benchmarks, do <code>@bors try @rust-timer queue include=-doc</code>; the <code>-doc</code> matches the <code>-doc</code> appended to rustdoc benchmarks. <code>&lt;INCLUDE&gt;</code> is a comma-separated list of substrings; only one of the items in the list of substrings must match.</li>
<li><code>exclude=&lt;EXCLUDE&gt;</code> is similar to <code>include=</code>, but instead skips any benchmarks that have one of the items in the list of substrings as a substring of its name.</li>
<li><code>runs=&lt;RUNS&gt;</code> configures how many times the benchmark is run. <code>&lt;RUNS&gt;</code> is an integer.</li>
Copy link
Member

Choose a reason for hiding this comment

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

Do you know the default value?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would assume 1?

Copy link
Member

Choose a reason for hiding this comment

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

This is like cap-lints -- some benchmarks run once, some run 3 times by default (we generally take the minimum of the metrics), and by default there's not an override, so we use the configuration from the benchmark.

Copy link
Member Author

Choose a reason for hiding this comment

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

So you're saying that some benchmarks run multiple times, e.g. with different cap-lints settings? And this option will let you override that?

Copy link
Member

Choose a reason for hiding this comment

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

No, not with different cap-lints settings. I'm saying that this option forces a particular number of runs; without it set, the number of runs varies from benchmark to benchmark.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we're saying the same thing, but I must not have been clear enough. As I understand it, you're saying that by default different benchmarks have different numbers of runs (some might be 1, some 2, some 3, etc.), and if you use runs= you can make all the benchmarks have the same number of runs (e.g., 2).

Are there any benchmarks that have their default run count set to 0?

Copy link
Member

Choose a reason for hiding this comment

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

No, all benchmarks run at least once by default. I'm not sure if runs=0 would work, but it would also be pretty useless I'd expect.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added more info in 677a309, does that look good to you?

Both I and `@jyn514` had no idea that `@rust-timer queue exclude`
existed because there's no documentation, or at least I wasn't able to
find any publicized documentation.
@camelid
Copy link
Member Author

camelid commented Mar 2, 2021

CI failures look spurious.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

This looks great to me, left a few comments which I think could be integrated, and then we can merge.

@Mark-Simulacrum Mark-Simulacrum merged commit 10bf13b into rust-lang:master Mar 9, 2021
@camelid camelid deleted the help-page branch March 9, 2021 03:36
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