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

CI: use free runner in dist-aarch64-msvc #133190

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

MarcoIeni
Copy link
Member

similar to #133175

try-job: dist-aarch64-msvc

@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 18, 2024
@MarcoIeni MarcoIeni force-pushed the dist-aarch64-msvc-free branch from 40301e3 to 0270580 Compare November 18, 2024 21:09
@MarcoIeni
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 18, 2024
…<try>

CI: use free runner in dist-aarch64-msvc

try-job: dist-aarch64-msvc
@bors
Copy link
Contributor

bors commented Nov 18, 2024

⌛ Trying commit 0270580 with merge e47a940...

@bors
Copy link
Contributor

bors commented Nov 19, 2024

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

@MarcoIeni
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2024
…<try>

CI: use free runner in dist-aarch64-msvc

try-job: dist-aarch64-msvc
@bors
Copy link
Contributor

bors commented Nov 19, 2024

⌛ Trying commit 0270580 with merge 652db1f...

@MarcoIeni
Copy link
Member Author

The previous run took too long to build llvm. Hopefully the next run will be better because of a warm cache
image

image

@Kobzol
Copy link
Contributor

Kobzol commented Nov 19, 2024

r? @Kobzol

@rustbot rustbot assigned Kobzol and unassigned Mark-Simulacrum Nov 19, 2024
@bors
Copy link
Contributor

bors commented Nov 19, 2024

☀️ Try build successful - checks-actions
Build commit: 652db1f (652db1fa797c10ea1d25463923ba0bff4a82a536)

@klensy
Copy link
Contributor

klensy commented Nov 19, 2024

sccache not used for that job, but we can try to build manually and apply it here (or not).

@MarcoIeni
Copy link
Member Author

MarcoIeni commented Nov 19, 2024

From the logs, I can see that we build llvm 4 times. The first 2 times is free. The other times take 52 minutes.

I guess we do this for PGO reasons. Maybe in this case we can't use a cache for LLVM, therefore we can't use a free runner. Kobzol, what do you think? 🤔

@klensy
Copy link
Contributor

klensy commented Nov 19, 2024

A don't see opt-dist, so there no pgo?

Ahh, there 2 targets, thats why, probably --target=aarch64-pc-windows-msvc,arm64ec-pc-windows-msvc

LLVM_TARGET_ARCH=aarch64
LLVM_TARGET_ARCH=x86_64

@Kobzol
Copy link
Contributor

Kobzol commented Nov 19, 2024

Yeah there should be no PGO in this job. We only do that for x64 Linux and Windows.

@MarcoIeni
Copy link
Member Author

ok, interesting, thanks both!

I see two options here:

  • split this job in two, each one for each target (I'm not sure if there are consequences such as publishing two times some tools 🙈)
  • trying to setup llvm cache. But I'm not sure this would be enough since right now this job takes 3h 49 minutes. If we setup llvm cache, building llvm could go down to 10 minutes (from what I've seen). So 3h 49m - (52 - 10) * 2 = 2h 25m which is on the limit of what we can accept.

Anyway, setting llvm cache is already a win, even if we don't switch to free runners, because the large runner will run for less time. Can we setup llvm cache? If yes, how?

@klensy
Copy link
Contributor

klensy commented Nov 19, 2024

Weird, i see --enable-sccache enabled, but no output from it. I'll try run in #133033

@klensy
Copy link
Contributor

klensy commented Nov 19, 2024

Looked at both runs again: they both was on different image versions, so second one didn't reuse sccache, thats the reason.

compiler versions:
MSVC/14.41.34120
MSVC/14.42.34433

@MarcoIeni
Copy link
Member Author

Thanks for looking into this, I will trigger bors try again. 👍

@MarcoIeni
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Nov 19, 2024

⌛ Trying commit 0270580 with merge 6d2cc49...

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2024
…<try>

CI: use free runner in dist-aarch64-msvc

try-job: dist-aarch64-msvc
@bors
Copy link
Contributor

bors commented Nov 19, 2024

☀️ Try build successful - checks-actions
Build commit: 6d2cc49 (6d2cc495b75bbccbcbda898335d8551e521fdc99)

@MarcoIeni
Copy link
Member Author

Took 2h and 20m.
Jakub, imo this could be acceptable because it's under the average of 2h 30m. But it's near the limit of course.

Maybe we can merge it and monitor the auto builds in the next week?

@Kobzol
Copy link
Contributor

Kobzol commented Nov 19, 2024

I'd say the average is more like 3+ hours right now, based on Datadog metrics. So in that sense this duration should be fine.

@klensy
Copy link
Contributor

klensy commented Nov 19, 2024

Btw, is datadog filters out try builds internally? Because i see that data sent in try builds too.

@Kobzol
Copy link
Contributor

Kobzol commented Nov 19, 2024

It allows us to filter all of that, yeah. I was looking at just auto builds.

@MarcoIeni MarcoIeni marked this pull request as ready for review November 19, 2024 19:50
@MarcoIeni
Copy link
Member Author

I'd say the average is more like 3+ hours right now, based on Datadog metrics. So in that sense this duration should be fine.

Great! I put this as ready for review.

I will review what jobs are the bottleneck after this is merged anyway 👍

@Kobzol
Copy link
Contributor

Kobzol commented Nov 19, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Nov 19, 2024

📌 Commit 0270580 has been approved by Kobzol

It is now in the queue for this repository.

@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 Nov 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 20, 2024
Rollup of 5 pull requests

Successful merges:

 - rust-lang#132732 (Use attributes for `dangling_pointers_from_temporaries` lint)
 - rust-lang#133108 (lints_that_dont_need_to_run: never skip future-compat-reported lints)
 - rust-lang#133190 (CI: use free runner in dist-aarch64-msvc)
 - rust-lang#133196 (Make rustc --explain compatible with BusyBox less)
 - rust-lang#133216 (Implement `~const Fn` trait goal in the new solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c364ff7 into rust-lang:master Nov 20, 2024
7 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 20, 2024
Rollup merge of rust-lang#133190 - MarcoIeni:dist-aarch64-msvc-free, r=Kobzol

CI: use free runner in dist-aarch64-msvc

try-job: dist-aarch64-msvc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants