Skip to content

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 20, 2023

This removes the dist-x86_64-apple-alt build to reduce CI usage because I suspect nobody is using it. This builder is almost identical to the dist-x86_64-apple with the small difference that the latter adds rust.lto=thin, and I do not think that difference was intentional. The reason they are identical is because llvm assertions were disabled in #44610, but I did not see any discussion about the consequence that this made the alt build identical to the normal build. Perhaps because #44610 was intended to be temporary?

@rustbot
Copy link
Collaborator

rustbot commented Aug 20, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@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 Aug 20, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Aug 20, 2023

Just to compare the configure settings, the alt build differs by:

-build.sanitizers     := True
-rust.codegen-backends := ['llvm']
-rust.lld             := True
-rust.llvm-tools      := True
-rust.lto             := thin

none of which seem particularly important.

@Mark-Simulacrum
Copy link
Member

This makes sense to me, good find. I think given current goals around optimizing capacity, I'm inclined to remove as well, we might restore with re-enabled assertions later (with faster macos builders or something else).

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 20, 2023

📌 Commit 889b55b has been approved by Mark-Simulacrum

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 Aug 20, 2023
@bors
Copy link
Collaborator

bors commented Aug 21, 2023

⌛ Testing commit 889b55b with merge c60ff10...

@bors
Copy link
Collaborator

bors commented Aug 21, 2023

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing c60ff10 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 21, 2023
@bors bors merged commit c60ff10 into rust-lang:master Aug 21, 2023
@rustbot rustbot added this to the 1.74.0 milestone Aug 21, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c60ff10): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.2% [4.2%, 4.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 635.074s -> 636.639s (0.25%)
Artifact size: 346.88 MiB -> 346.94 MiB (0.02%)

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 merged-by-bors This PR was explicitly merged by bors. 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.

5 participants