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

Add even more GHA log groups #113514

Merged
merged 13 commits into from
Jul 15, 2023
Merged

Add even more GHA log groups #113514

merged 13 commits into from
Jul 15, 2023

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 9, 2023

This also adds a dynamic check that we don't emit nested groups, since GHA currently doesn't support them.

r? @oli-obk

@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-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. labels Jul 9, 2023
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514 jyn514 force-pushed the more-gha-groups branch from 2c8b083 to 7064af0 Compare July 10, 2023 04:08
oli-obk
oli-obk reviewed Jul 10, 2023
View reviewed changes
src/tools/build_helper/src/ci.rs
Comment on lines +52 to +57
// https://github.com/actions/toolkit/issues/1001
assert!(
!GROUP_ACTIVE.swap(true, Ordering::Relaxed),
"nested groups are not supported by GHA!"
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should probably do this in ui_test, too

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2023

@bors r+ rollup

wow, logs look so accessible now

@bors
Copy link
Contributor

bors commented Jul 10, 2023

📌 Commit 7064af0 has been approved by oli-obk

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 10, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 10, 2023
@matthiaskrgr
Rollup merge of rust-lang#113514 - jyn514:more-gha-groups, r=oli-obk
a3029e2 
Add even more GHA log groups

This also adds a dynamic check that we don't emit nested groups, since GHA currently doesn't support them.

r? `@oli-obk`
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 10, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 10, 2023
@matthiaskrgr
Rollup merge of rust-lang#113514 - jyn514:more-gha-groups, r=oli-obk
58f26fe 
Add even more GHA log groups

This also adds a dynamic check that we don't emit nested groups, since GHA currently doesn't support them.

r? ``@oli-obk``
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 10, 2023
Closed
@matthiaskrgr
Copy link
Member

@bors r-
#113554 (comment)

@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 10, 2023
@bors
Copy link
Contributor

bors commented Jul 12, 2023

☔ The latest upstream changes (presumably #113214) made this pull request unmergeable. Please resolve the merge conflicts.

@jyn514 jyn514 force-pushed the more-gha-groups branch from 7064af0 to db2853f Compare July 13, 2023 05:41
@jyn514
Copy link
Member Author

jyn514 commented Jul 13, 2023

@bors r=oli-obk rollup=iffy

@bors
Copy link
Contributor

bors commented Jul 13, 2023

📌 Commit db2853fe97f9897a3374d0f367eed48db0520619 has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 13, 2023
@bors
Copy link
Contributor

bors commented Jul 14, 2023

☔ The latest upstream changes (presumably #113676) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 14, 2023
@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 14, 2023
@bors
Copy link
Contributor

bors commented Jul 14, 2023

📌 Commit 3a0caed has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 14, 2023
@bors
Copy link
Contributor

bors commented Jul 14, 2023

⌛ Testing commit 3a0caed with merge 4888db91469c3e293a7ad1ba5ceae2649821da6c...

@bors
Copy link
Contributor

bors commented Jul 15, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 4888db91469c3e293a7ad1ba5ceae2649821da6c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 15, 2023
@bors
Copy link
Contributor

bors commented Jul 15, 2023

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4888db91469c3e293a7ad1ba5ceae2649821da6c): 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)

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)		 2.0% [0.8%, 3.3%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.6% [-3.7%, -1.6%] 2
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.3% [1.6%, 4.9%] 2
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 3.3% [1.6%, 4.9%] 2

Binary size

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

Bootstrap: 657.64s -> 659.449s (0.28%)

@jyn514
Copy link
Member Author

jyn514 commented Jul 15, 2023

try build got merged and this was tested before the tree was closed 😞 bef6ff6 https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Cannot.20get.20rust-dev-nightly.20from.20ci-artifacts.2Erust-lang.2Eorg

@jyn514
Copy link
Member Author

jyn514 commented Jul 15, 2023

@bors retry #113713

@bors
Copy link
Contributor

bors commented Jul 15, 2023

⌛ Testing commit 3a0caed with merge 4d6e426...

@bors
Copy link
Contributor

bors commented Jul 15, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 4d6e426 to master...

@bors bors merged commit 4d6e426 into rust-lang:master Jul 15, 2023
@rustbot rustbot added this to the 1.73.0 milestone Jul 15, 2023
This was referenced Jul 15, 2023
Merged
Merged
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4d6e426): 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
Regressions ❌
(secondary)		 2.9% [2.7%, 3.0%] 6
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)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.3% [-3.6%, -1.7%] 3
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)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.1% [-2.1%, -2.1%] 1
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 657.64s -> 657.725s (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jul 15, 2023
@jyn514 jyn514 deleted the more-gha-groups branch July 15, 2023 17:13
@jyn514
Copy link
Member Author

jyn514 commented Jul 15, 2023

this doesn't affect how the compiler was built at all, the perf regression must be spurious.

This was referenced Jul 16, 2023
Merged
Merged
Closed
@pnkfelix
Copy link
Member

this doesn't affect how the compiler was built at all, the perf regression must be spurious.

I agree. 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
@onur-ozkan onur-ozkan mentioned this pull request Jul 25, 2023
Merged
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
8 participants
@jyn514 @rust-log-analyzer @oli-obk @bors @matthiaskrgr @rust-timer @pnkfelix @rustbot