Skip to content

Less import overhead for errors #107679

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 7 commits into from
Feb 5, 2023
Merged

Conversation

est31
Copy link
Member

@est31 est31 commented Feb 5, 2023

This removes huge (3+ lines) import lists found in files that had their error reporting migrated. These lists are bad for developer workflows as adding, removing, or editing a single error's name might cause a chain reaction that bloats the git diff. As the error struct names are long, the likelihood of such chain reactions is high.

Follows the suggestion by @Nilstrieb in the zulip thread to replace the use errors::{FooErr, BarErr}; with use errors; and then changing to errors::FooErr on the usage sites.

I have used sed to do most of the changes, i.e. something like:

sed -i -E 's/(create_err|create_feature_err|emit_err|create_note|emit_fatal|emit_warning)\(([[:alnum:]]+|[A-Z][[:alnum:]:]*)( \{|\))/\1(errors::\2\3/' path/to/file.rs

& then I manually fixed the errors that occured. Most manual changes were required in compiler/rustc_parse/src/parser/expr.rs.

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 5, 2023
@est31 est31 force-pushed the less_import_overhead branch from 00f867b to 580cc89 Compare February 5, 2023 03:01
@compiler-errors
Copy link
Member

Cool, thanks @est31 for doing this.

@bors r+ p=1

@bors
Copy link
Collaborator

bors commented Feb 5, 2023

📌 Commit 580cc89 has been approved by compiler-errors

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 Feb 5, 2023
@bors
Copy link
Collaborator

bors commented Feb 5, 2023

⌛ Testing commit 580cc89 with merge 7f97aea...

@bors
Copy link
Collaborator

bors commented Feb 5, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 7f97aea to master...

1 similar comment
@bors
Copy link
Collaborator

bors commented Feb 5, 2023

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 7f97aea to master...

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (7f97aea): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4% [0.3%, 0.5%] 2
Regressions ❌
(secondary)
0.9% [0.3%, 1.3%] 11
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.7% [-0.7%, -0.7%] 2
All ❌✅ (primary) 0.4% [0.3%, 0.5%] 2

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.4% [2.3%, 2.5%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.4% [-6.0%, -2.5%] 4
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)
-4.8% [-4.9%, -4.6%] 2
All ❌✅ (primary) - - 0

@rustbot rustbot added the perf-regression Performance regression. label Feb 5, 2023
@lqd
Copy link
Member

lqd commented Feb 6, 2023

This is of course noise, and has corrected in #102842 (comment)

@rustbot label: +perf-regression-triaged

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants