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

Skip documentation for tier 2 targets on dist-x86_64-apple-darwin #90100

Merged
merged 3 commits into from
Oct 24, 2021

Conversation

Mark-Simulacrum
Copy link
Member

I don't have an easy way to test this locally, but I believe it should work. Based on one log result should shave ~14 minutes off the dist-x86_64-apple builder (doesn't help with aarch64 dist or x86_64 test builder, so not actually decreasing total CI time most likely).

r? @pietroalbini

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 20, 2021
@Mark-Simulacrum
Copy link
Member Author

Let me actually get this into a try build to test it

@Mark-Simulacrum
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Oct 20, 2021

⌛ Trying commit 3eae37c4c6795401a15a07028d607a690842ccad with merge a2b2936730df129423af44dab918d380df6ccdd4...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Oct 20, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 20, 2021
@Mark-Simulacrum
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Oct 20, 2021

⌛ Trying commit 5503dd9 with merge c18b3fe6f6448517cfb25f8facf58bfdb46016ad...

@bors
Copy link
Contributor

bors commented Oct 20, 2021

☀️ Try build successful - checks-actions
Build commit: c18b3fe6f6448517cfb25f8facf58bfdb46016ad (c18b3fe6f6448517cfb25f8facf58bfdb46016ad)

@Mark-Simulacrum
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Oct 21, 2021

⌛ Trying commit 9296efe with merge 8a9d748d49be179776d8e74ef8b372c75449e57e...

@bors
Copy link
Contributor

bors commented Oct 21, 2021

☀️ Try build successful - checks-actions
Build commit: 8a9d748d49be179776d8e74ef8b372c75449e57e (8a9d748d49be179776d8e74ef8b372c75449e57e)

@pietroalbini
Copy link
Member

r=me once the try commit is removed.

@Mark-Simulacrum
Copy link
Member Author

Well, try commits so far are yielding regressions in performance I think so deeper investigation necessary :)

@pietroalbini
Copy link
Member

Well the r=me was mostly about the code change itself 😅

@Mark-Simulacrum
Copy link
Member Author

Looks like this currently adds ~20 minutes to the time across try runs which seem to be just noisy neighbors or something -- all the times for other jobs went up. However, I've checked that the artifacts produced look good and we are cutting out the fairly costly docs building.

So I think we should probably go ahead and merge this; unfortunately we don't currently really track CI times anywhere, so it'll be a little hard to get a sense of whether this is an improvement, but I think the potential time savings are worth it. I also have an idea for how to track those that I'm going to experiment with a little today, but it'll be able to back-collect trivially.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 23, 2021
@Mark-Simulacrum
Copy link
Member Author

OK, I have a local setup for monitoring CI times now that should hopefully work well enough that we can detect changes caused by this PR (given the expected multi-minute impact).

I'm going to go ahead and approve (per earlier r=me) and will check in on the auto branch builds in a few days to see what the effect of this PR is. My guess is it'll help, but we'll see.

@bors r=pietroalbini

For the record, currently seeing (rolling 32 commit median time):

image

@bors
Copy link
Contributor

bors commented Oct 24, 2021

📌 Commit 9296efe has been approved by pietroalbini

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 24, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 24, 2021
…ietroalbini

Skip documentation for tier 2 targets on dist-x86_64-apple-darwin

I don't have an easy way to test this locally, but I believe it should work. Based on one log result should shave ~14 minutes off the dist-x86_64-apple builder (doesn't help with aarch64 dist or x86_64 test builder, so not actually decreasing total CI time most likely).

r? `@pietroalbini`
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Oct 24, 2021

Got an "attempt to subtract with overfow" error during std documentation in a rollup, could this be related to this pr?
#90222 (comment)

Edit: nevermind, #90042 (comment) had the same problem and it didn't contain this pr

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 24, 2021
…ietroalbini

Skip documentation for tier 2 targets on dist-x86_64-apple-darwin

I don't have an easy way to test this locally, but I believe it should work. Based on one log result should shave ~14 minutes off the dist-x86_64-apple builder (doesn't help with aarch64 dist or x86_64 test builder, so not actually decreasing total CI time most likely).

r? `@pietroalbini`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 24, 2021
…ietroalbini

Skip documentation for tier 2 targets on dist-x86_64-apple-darwin

I don't have an easy way to test this locally, but I believe it should work. Based on one log result should shave ~14 minutes off the dist-x86_64-apple builder (doesn't help with aarch64 dist or x86_64 test builder, so not actually decreasing total CI time most likely).

r? ``@pietroalbini``
This was referenced Oct 24, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#89558 (Add rustc lint, warning when iterating over hashmaps)
 - rust-lang#90100 (Skip documentation for tier 2 targets on dist-x86_64-apple-darwin)
 - rust-lang#90155 (Fix alignment of method headings for scannability)
 - rust-lang#90162 (Mark `{array, slice}::{from_ref, from_mut}` as const fn)
 - rust-lang#90221 (Fix ICE when forgetting to `Box` a parameter to a `Self::func` call)
 - rust-lang#90234 (Temporarily turn overflow checks off for rustc-rayon-core)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d814af9 into rust-lang:master Oct 24, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 24, 2021
@Mark-Simulacrum Mark-Simulacrum deleted the speed-macos-ci branch October 24, 2021 19:36
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 25, 2021
…bini

Move back to linux builder on try builds

Apparently deleted the wrong line when trying to revert changes to try in rust-lang#90100 which I now see still contains the do not merge commit -- maybe I forgot to force push the local changes I had pending or something.

r? `@pietroalbini`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants