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 regression test for #86106 #106036

Merged
merged 3 commits into from
Jan 7, 2023
Merged

Add regression test for #86106 #106036

merged 3 commits into from
Jan 7, 2023

Conversation

JohnTitor
Copy link
Member

Closes #86106
r? @nikic

Signed-off-by: Yuki Okushi jtitor@2k36.org

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2022
nikic
nikic reviewed Dec 22, 2022
View reviewed changes
src/test/codegen/issue-86106.rs Outdated Show resolved Hide resolved
src/test/codegen/issue-86106.rs Outdated Show resolved Hide resolved
src/test/codegen/issue-86106.rs Outdated Show resolved Hide resolved
src/test/codegen/issue-86106.rs Outdated Show resolved Hide resolved
@nikic
Copy link
Contributor

nikic commented Dec 22, 2022

@bors r+ rollup=never codegen test

@bors
Copy link
Contributor

bors commented Dec 22, 2022

📌 Commit 3b16aea has been approved by nikic

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 Dec 22, 2022
@bors
Copy link
Contributor

bors commented Dec 22, 2022

⌛ Testing commit 3b16aea with merge 9d5ce2cbfe667a67a118e7bf61170e8a52f161e2...

@bors
Copy link
Contributor

bors commented Dec 22, 2022

💔 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 Dec 22, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@JohnTitor
Copy link
Member Author

Is there a way to avoid running the test on nopt or force optimization?

@nikic
Copy link
Contributor

nikic commented Dec 23, 2022

-C opt-level=3 already forces optimization. From the output, it's not completely clear why the test fails. I've submitted #106078 to make this easier to debug in the future.

Possibly it's just a function merging failure. You could try passing -Z merge-functions=disabled and test the result of that (without the alias).

jyn514 added a commit to jyn514/rust that referenced this pull request Dec 31, 2022
@jyn514
Rollup merge of rust-lang#106036 - JohnTitor:issue-86106, r=nikic
900a2cc 
Add regression test for rust-lang#86106

Closes rust-lang#86106
r? `@nikic`

Signed-off-by: Yuki Okushi <jtitor@2k36.org>
@jyn514 jyn514 mentioned this pull request Dec 31, 2022
Closed
@JohnTitor
Add regression test for rust-lang#86106
75f1dfc 
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
@JohnTitor
Apply review suggestions
78e11a4 
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
@JohnTitor
Apply merge-functions=disabled
ab9313e 
Signed-off-by: Yuki Okushi <jtitor@2k36.org>
@JohnTitor
Copy link
Member Author

Sounds good, applied it: ab9313e

@nikic
Copy link
Contributor

nikic commented Jan 7, 2023

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Jan 7, 2023

📌 Commit ab9313e has been approved by nikic

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

bors commented Jan 7, 2023

⌛ Testing commit ab9313e with merge ee0412d...

@bors
Copy link
Contributor

bors commented Jan 7, 2023

☀️ Test successful - checks-actions
Approved by: nikic
Pushing ee0412d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 7, 2023
@bors bors merged commit ee0412d into rust-lang:master Jan 7, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 7, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ee0412d): comparison URL.

Overall result: ✅ improvements - 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
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.7% [-0.7%, -0.7%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

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

Cycles

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

@JohnTitor JohnTitor deleted the issue-86106 branch June 25, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikic nikic nikic left review comments

Assignees

@nikic nikic

Labels
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.
Projects
None yet
Milestone
1.68.0
Development

Successfully merging this pull request may close these issues.

String::new() is slower than "".to_string()
6 participants
@JohnTitor @nikic @bors @rust-log-analyzer @rust-timer @rustbot