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.

Sign up for GitHub

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

Jump to bottom

Rollup of 3 pull requests #113738

Merged
merged 8 commits into from
Jul 16, 2023
Merged

Rollup of 3 pull requests #113738

merged 8 commits into from
Jul 16, 2023

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 15, 2023

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

jyn514 added 8 commits July 15, 2023 12:27
@jyn514
Deduplicate Builder::try_run and mark Config::try_run as deprecated
63d7992 
This does three things:
1. Remove `forward!(Build, fn try_run())`. Having `try_run` behave differently as a free function than an associated function is confusing, and `Builder::try_run` is a very desirable name.
2. Move `test::try_run` and `run::try_run` to `Builder::try_run`. These functions are different than `Config::try_run` - they delay the failure and print it out at the end of the build.
3. Mark `Config::try_run` as deprecated to encourage people to use `Builder::try_run` instead.
@jyn514
Rename Builder::try_run to run_delaying_failure
78f51a4
@jyn514
Replace builder::try_run_quiet with run_quiet_delaying_failure
c0c6a24 
It was only used when a `builder` is available, and I want to encourage using the version that supports `--no-fail-fast`.
@jyn514
Remove unused bootstrap::util::CiEnv enum
6f589d5 
the right one is `build_helper::CiEnv`; this one wasn't even used.
@jyn514
update mailmap for myself
2d3a964
@jyn514
Rollup merge of rust-lang#113643 - jyn514:try-run, r=ozkanonur
90c74e2 
bootstrap: Clean up try_run

r? `@ozkanonur` since you reviewed `@GuillaumeGomez's` PR

i recommend reviewing commit-by-commit
@jyn514
Rollup merge of rust-lang#113731 - jyn514:ci-env, r=ozkanonur
0a299f6 
Remove unused `bootstrap::util::CiEnv` enum

the right one is `build_helper::CiEnv`; this one wasn't even used.
@jyn514
Rollup merge of rust-lang#113737 - jyn514:mailmap, r=jyn514
df729c2 
update mailmap for myself
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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) rollup A PR which is a rollup labels Jul 15, 2023
@jyn514
Copy link
Member Author

jyn514 commented Jul 15, 2023

@bors r+ p=3 rollup=never

@bors
Copy link
Contributor

bors commented Jul 15, 2023

📌 Commit df729c2 has been approved by jyn514

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 Jul 15, 2023
@bors
Copy link
Contributor

bors commented Jul 16, 2023

⌛ Testing commit df729c2 with merge 2c718d1...

@bors
Copy link
Contributor

bors commented Jul 16, 2023

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 2c718d1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 16, 2023
@bors bors merged commit 2c718d1 into rust-lang:master Jul 16, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 16, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#113643 bootstrap: Clean up try_run 736696ab8e82b33a61d94c80ccf937f8d58128ff (link)
#113731 Remove unused bootstrap::util::CiEnv enum cb5f6386cd01c3a19aa24bf561e9d3a9d6aa08e5 (link)
#113737 update mailmap for myself c47d21f7c26ba9fc7052a9a167e34d4f0652a748 (link)

previous master: 4124617c6e

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2c718d1): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.7% [0.7%, 0.7%] 1
Regressions ❌
(secondary)		 3.1% [2.7%, 3.5%] 6
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.7% [0.7%, 0.7%] 1

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)		 1.5% [1.1%, 2.1%] 6
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Cycles

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)		 3.4% [3.4%, 3.4%] 1
Regressions ❌
(secondary)		 2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 3.4% [3.4%, 3.4%] 1

Binary size

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

Bootstrap: 656.574s -> 657.636s (0.16%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 16, 2023
@jyn514 jyn514 deleted the rollup-mjcya4c branch July 16, 2023 12:15
@jyn514
Copy link
Member Author

jyn514 commented Jul 16, 2023

this is almost certainly spurious - the last two PRs don't affect code at all and the first one shouldn't have affected anything other than maybe env variables in tests. see #113514 (comment)

@pnkfelix
Copy link
Member

I agree with @jyn514 . Marking as triaged.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup 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)
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
5 participants
@jyn514 @bors @rust-timer @pnkfelix @rustbot