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

Describe how to trigger perf runs #1237

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Describe how to trigger perf runs #1237

merged 1 commit into from
Oct 21, 2021

Conversation

lqd
Copy link
Member

@lqd lqd commented Oct 20, 2021

This PR adds the command to request perf runs on PRs.

Fixes #1236

@JohnTitor
Copy link
Member

I'm not sure if the profiling chapter is the best place to describe in details. We have https://rustc-dev-guide.rust-lang.org/getting-started.html#performance and it would be a better one, I think. And I feel like that it's more of instruction for team members rather than (new) contributors so forge or something else would be better, though it's probably fine to mention here.

@spastorino
Copy link
Member

I'm unsure about the getting started section, maybe it can go there too but as I was reading the place where @lqd has placed this it made sense to me because it was already referring to running perf with the bot, it's just that @lqd was explicit about how to run it.

@lqd
Copy link
Member Author

lqd commented Oct 20, 2021

Right, this page already mentioned the old bot, and why it seemed an appropriate one.

@JohnTitor
Copy link
Member

Uhm, indeed the getting started is not something that team members (i.e. someone has the r+ right) read usually. But I still feel like the profiling chapter is a bit too specific to mention the general bot usage in detail and I don't expect anyone can find that information.

@spastorino
Copy link
Member

spastorino commented Oct 20, 2021

I think we should split this discussion into two. On one side maybe we need to restructure some stuff to properly fit what @JohnTitor is saying on the other side I think this PR is already a simple improvement given first, that we don't have this information anywhere in the guide, and that the section is already mentioning it. The other option is to make a very minimal change, like ...

  • The rustc-perf project makes this easy and can be triggered to run on a PR via the @rust-timer bot with @bors try @rust-timer queue.

@camelid
Copy link
Member

camelid commented Oct 20, 2021

rustc-perf has a help page that we could consider linking to: https://perf.rust-lang.org/help.html

src/profiling.md Outdated Show resolved Hide resolved
@JohnTitor
Copy link
Member

Ah, makes sense, restructuring can be later 👍 I'm fine with the current or the link @camelid provided.

@jyn514
Copy link
Member

jyn514 commented Oct 20, 2021

And I feel like that it's more of instruction for team members rather than (new) contributors so forge or something else would be better

There are already quite a lot of docs that aren't meant for new contributors, I'm not sure how that's related to it being on forge or not. Most of the profiling section is for existing contributors.

@JohnTitor
Copy link
Member

There are already quite a lot of docs that aren't meant for new contributors, I'm not sure how that's related to it being on forge or not. Most of the profiling section is for existing contributors.

I mean, perf-run requires a try right on rust-lang/rust so it's not for existing contributors but team members.

@spastorino
Copy link
Member

I'm fine with the current state of this, would leave the final decision to someone else :).

Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

👍, let's go ahead, thanks!

@JohnTitor JohnTitor merged commit 87968b4 into rust-lang:master Oct 21, 2021
@lqd lqd deleted the perf-bot branch October 21, 2021 16:01
Kobzol pushed a commit to Kobzol/rustc-dev-guide that referenced this pull request Jan 3, 2025
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.

Document how to run perf on PRs
5 participants