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

handle forced compiler and revert #137476 #138039

Merged
merged 4 commits into from
Mar 6, 2025

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Mar 5, 2025

Fixes #138004

I would appreciate it if we could measure CI pipelines with the current changes to see if this reduces recent CI overhead. cc @rust-lang/infra

try-job: dist-powerpc64le-linux

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2025

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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 5, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Mar 5, 2025

@onur-ozkan you could try-job the slowest job for a rough measurement (the dist ppcle64 or whatever it's called) that we know regressed in build time

@onur-ozkan
Copy link
Member Author

We don't build tools on try-jobs, but I can add a small patch to force that for testing.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 5, 2025

Yeah, it was the powerpc64 le one, I think (not on PC right now).

@jieyouxu
Copy link
Member

jieyouxu commented Mar 5, 2025

Ah right, I forgot about the try-job tool build distinction again...

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Mar 5, 2025
@onur-ozkan onur-ozkan force-pushed the handle-forced-compiler-on-tools branch from ae78953 to cd0f743 Compare March 5, 2025 07:13
@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2025

Some changes occurred in src/tools/opt-dist

cc @Kobzol

@onur-ozkan
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
…-tools, r=<try>

handle forced compiler and revert rust-lang#137476

Fixes rust-lang#138004

I would appreciate it if we could measure CI pipelines with the current changes to see if this reduces recent CI overhead. cc `@rust-lang/infra`

try-job: powerpc64le-unknown-linux-gnu
@bors
Copy link
Contributor

bors commented Mar 5, 2025

⌛ Trying commit cd0f743 with merge b46218a...

@rust-log-analyzer

This comment has been minimized.

@onur-ozkan
Copy link
Member Author

Messed up with the job name on the previous call. @bors try

@bors
Copy link
Contributor

bors commented Mar 5, 2025

⌛ Trying commit cd0f743 with merge dcee436...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
…-tools, r=<try>

handle forced compiler and revert rust-lang#137476

Fixes rust-lang#138004

I would appreciate it if we could measure CI pipelines with the current changes to see if this reduces recent CI overhead. cc `@rust-lang/infra`

try-job: dist-powerpc64le-linux
@rust-log-analyzer

This comment was marked as outdated.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 5, 2025

Try builds are different only for jobs that use the PGO/BOLT optimization pipeline. For the PowerPC job, try job is exactly the same as an auto job.

@onur-ozkan
Copy link
Member Author

onur-ozkan commented Mar 5, 2025

x86_64-gnu-llvm-18 failed two times in a row due to network specific problem on Gcc step.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 5, 2025

Yeah, we might want to host the GCC dependencies on our mirrors, I'll open an infra topic.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 5, 2025

For reference, the job duration was ~2h45m before #137476, and ~3h15m after it.

@Kobzol
Copy link
Contributor

Kobzol commented Mar 5, 2025

I have a script that compares the executed bootstrap stages (later I want to add this functionality to #138013), I'll run it on the output once it finishes. So far it seems to be on the slower side though :/

@bors
Copy link
Contributor

bors commented Mar 5, 2025

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

@onur-ozkan
Copy link
Member Author

Maybe it's not just about #137476? We shouldn't get the build overhead of #137476 in this PR.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@bors
Copy link
Contributor

bors commented Mar 6, 2025

📌 Commit 64dd484 has been approved by jieyouxu

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 6, 2025

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Mar 6, 2025
@jieyouxu jieyouxu assigned jieyouxu and unassigned Mark-Simulacrum Mar 6, 2025
@bors
Copy link
Contributor

bors commented Mar 6, 2025

⌛ Testing commit 64dd484 with merge 0e29361...

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 6, 2025
…-tools, r=jieyouxu

handle forced compiler and revert rust-lang#137476

Fixes rust-lang#138004

I would appreciate it if we could measure CI pipelines with the current changes to see if this reduces recent CI overhead. cc `@rust-lang/infra`

try-job: dist-powerpc64le-linux
@ehuss
Copy link
Contributor

ehuss commented Mar 6, 2025

(Not a blocker) With this PR, it still seems like it is building the compiler an extra unnecessary time:

Building compiler artifacts (stage1:x86_64-unknown-linux-gnu -> stage2:powerpc64le-unknown-linux-gnu)
Building compiler artifacts (stage0:x86_64-unknown-linux-gnu -> stage1:powerpc64le-unknown-linux-gnu)

I would not expect that second build to the host architecture. I would expect that "stage1" compiler to not be used at all (it is not used for compiling anything, and it is not included in the dist artifacts).

Do we know why that is happening?

@Kobzol
Copy link
Contributor

Kobzol commented Mar 6, 2025

(Just noting that this might explain the "missing" 15 minutes).

@jieyouxu
Copy link
Member

jieyouxu commented Mar 6, 2025

(Not a blocker) With this PR, it still seems like it is building the compiler an extra unnecessary time:

Building compiler artifacts (stage1:x86_64-unknown-linux-gnu -> stage2:powerpc64le-unknown-linux-gnu)
Building compiler artifacts (stage0:x86_64-unknown-linux-gnu -> stage1:powerpc64le-unknown-linux-gnu)

I would not expect that second build to the host architecture. I would expect that "stage1" compiler to not be used at all (it is not used for compiling anything, and it is not included in the dist artifacts).

Do we know why that is happening?

@ehuss can you share what config.toml you used, and the invocation you used (I ask because ./x dist with no args can differ vs ./x dist with path filters)? I can look into it.

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-mingw-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
failures:

---- [mir-opt] tests\mir-opt\slice_drop_shim.rs stdout ----

error: compilation failed!
status: exit code: 0xc00000fd
command: PATH="D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin;D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0-bootstrap-tools\x86_64-pc-windows-gnu\release\deps;D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage0\bin;D:\a\rust\rust\ninja;D:\a\rust\rust\mingw64\bin;C:\msys64\usr\bin;D:\a\rust\rust\sccache;C:\Program Files\MongoDB\Server\5.0\bin;C:\aliyun-cli;C:\vcpkg;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\3.3.1\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 8.0\bin;C:\Program Files\R\R-4.4.2\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\SeleniumWebDrivers\ChromeDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.21.13\x64\bin;C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;C:\hostedtoolcache\windows\Python\3.9.13\x64;C:\hostedtoolcache\windows\Ruby\3.0.7\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.442-6\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\ProgramData\Chocolatey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\Program Files\dotnet;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\TortoiseSVN\bin;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files\Microsoft SQL Server\150\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\WiX Toolset v3.14\bin;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\140\DTS\Binn;C:\Program Files\Microsoft SQL Server\150\DTS\Binn;C:\Program Files\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.9.9\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2\\bin\\rustc.exe" "D:\\a\\rust\\rust\\tests\\mir-opt\\slice_drop_shim.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=C:\\Users\\runneradmin\\.cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=D:\\a\\rust\\rust\\vendor" "--sysroot" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\stage2" "--target=x86_64-pc-windows-gnu" "--check-cfg" "cfg(test,FALSE)" "-O" "-Copt-level=1" "-Zdump-mir=AddMovesForPackedDrops | AddMovesForPackedDrops" "-Zvalidate-mir" "-Zlint-mir" "-Zdump-mir-exclude-pass-number" "-Zmir-include-spans=false" "--crate-type=rlib" "-Zmir-opt-level=4" "-Zmir-enable-passes=+ReorderBasicBlocks,+ReorderLocals" "-Zdump-mir-dir=D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\mir-opt\\slice_drop_shim" "--emit" "mir" "-C" "prefer-dynamic" "--out-dir" "D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\test\\mir-opt\\slice_drop_shim" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=D:\\a\\rust\\rust\\build\\x86_64-pc-windows-gnu\\native\\rust-test-helpers" "-Zmir-opt-level=0" "-Clink-dead-code"
--- stderr -------------------------------
warning: function `main` is never used
##[warning] --> D:\a\rust\rust\tests\mir-opt\slice_drop_shim.rs:9:4
  |
---
test result: FAILED. 324 passed; 1 failed; 3 ignored; 0 measured; 0 filtered out; finished in 12.84s

Some tests failed in compiletest suite=mir-opt mode=mir-opt host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
Build completed unsuccessfully in 1:36:44
make: *** [Makefile:126: ci-mingw-x] Error 1
  network time: Thu, 06 Mar 2025 16:02:06 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

@bors
Copy link
Contributor

bors commented Mar 6, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 6, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Mar 6, 2025

STATUS_STACK_OVERFLOW 🤔 Let me run this locally...

@jieyouxu
Copy link
Member

jieyouxu commented Mar 6, 2025

I can't seem to repro this locally on mingw. Let's see if it's spurious; I'll open an issue to track this.

EDIT: #138110

@bors retry

@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 Mar 6, 2025
@bors
Copy link
Contributor

bors commented Mar 6, 2025

⌛ Testing commit 64dd484 with merge e6af292...

@onur-ozkan
Copy link
Member Author

(Not a blocker) With this PR, it still seems like it is building the compiler an extra unnecessary time:


Building compiler artifacts (stage1:x86_64-unknown-linux-gnu -> stage2:powerpc64le-unknown-linux-gnu)

Building compiler artifacts (stage0:x86_64-unknown-linux-gnu -> stage1:powerpc64le-unknown-linux-gnu)

I would not expect that second build to the host architecture. I would expect that "stage1" compiler to not be used at all (it is not used for compiling anything, and it is not included in the dist artifacts).

Do we know why that is happening?

Yeah, we can avoid that too, but are you sure that this wasn't happening before #137476?

@ehuss
Copy link
Contributor

ehuss commented Mar 6, 2025

I posted my notes into #138123. I only compared after this PR, and before #137215, and only with a few configs.

@bors
Copy link
Contributor

bors commented Mar 6, 2025

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing e6af292 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 6, 2025
@bors bors merged commit e6af292 into rust-lang:master Mar 6, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 6, 2025
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e6af292): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (secondary -2.4%)

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)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.9% [-4.1%, -3.7%] 3
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 773.363s -> 774.453s (0.14%)
Artifact size: 362.14 MiB -> 362.07 MiB (-0.02%)

@onur-ozkan onur-ozkan deleted the handle-forced-compiler-on-tools branch March 7, 2025 06:05
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-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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.

Investigate cross-compile dist tool flow incorrectly building {host,build} compilers
9 participants