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

Refactor lower_stmts #87773

Closed
wants to merge 1 commit into from
Closed

Refactor lower_stmts #87773

wants to merge 1 commit into from

Conversation

camsteffen
Copy link
Contributor

I have this change in #87688 but I think it deserves its own PR and a perf run.

Changes lower_stmt to lower_stmts and avoid creating a SmallVec for each ast::Stmt.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 4, 2021
@camsteffen
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 4, 2021
@bors
Copy link
Contributor

bors commented Aug 4, 2021

⌛ Trying commit 28cf90d with merge 4e7f9014b4dc19485bb3ea6d3ebf68bab256b14f...

camsteffen
camsteffen commented Aug 4, 2021
View reviewed changes
compiler/rustc_ast_lowering/src/lib.rs
&mut self,
ast_stmts: &[Stmt],
) -> (&'hir [hir::Stmt<'hir>], Option<&'hir hir::Expr<'hir>>) {
let mut stmts = SmallVec::<[hir::Stmt<'hir>; 8]>::new();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if 8 is optimal, but I think it is already used in the current implementation, in alloc_from_iter.

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 4, 2021
@bors
Copy link
Contributor

bors commented Aug 4, 2021

☀️ Try build successful - checks-actions
Build commit: 4e7f9014b4dc19485bb3ea6d3ebf68bab256b14f (4e7f9014b4dc19485bb3ea6d3ebf68bab256b14f)

@rust-timer
Copy link
Collaborator

Queued 4e7f9014b4dc19485bb3ea6d3ebf68bab256b14f with parent 6fe0886, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (4e7f9014b4dc19485bb3ea6d3ebf68bab256b14f): comparison url.

Summary: This benchmark run did not return any significant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 4, 2021
@camsteffen
Copy link
Contributor Author

Neutral results I guess. Maybe it's easier to just close this PR.

@camsteffen camsteffen deleted the lower-stmts branch August 5, 2021 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@camsteffen @rust-highfive @rust-timer @bors @petrochenkov @rustbot