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

Reduce the size of hir::Expr. #58258

Merged
merged 1 commit into from
Feb 14, 2019
Merged

Reduce the size of hir::Expr. #58258

merged 1 commit into from
Feb 14, 2019

Conversation

nnethercote
Copy link
Contributor

From 104 bytes to 72 bytes on x86-64. This slightly reduces instruction
counts.

Also add an assertion about the size.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(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 Feb 7, 2019
@nnethercote
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 7, 2019

⌛ Trying commit fc75a4d62b37230ce6cb050952c19a8517d3cc67 with merge 387a2a54ba590de3ea8fadc38678835779598b9b...

@bors
Copy link
Contributor

bors commented Feb 7, 2019

☀️ Test successful - checks-travis
State: approved= try=True

@michaelwoerister
Copy link
Member

@rust-timer build 387a2a54ba590de3ea8fadc38678835779598b9b

@rust-timer
Copy link
Collaborator

Success: Queued 387a2a54ba590de3ea8fadc38678835779598b9b with parent 825f355, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 387a2a54ba590de3ea8fadc38678835779598b9b

@nnethercote
Copy link
Contributor Author

Changes to instruction counts are negligible. Looks like the extra allocations required are balanced by whatever reductions in memory copying.

However, the max-rss reductions are real -- e.g. I used DHAT on a "Check CleanIncr" build of tuple-stress, and peak heap usage dropped from 271.1MB to 258.6MB.

@michaelwoerister
Copy link
Member

Looks good. Thanks, @nnethercote!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 8, 2019

📌 Commit fc75a4d62b37230ce6cb050952c19a8517d3cc67 has been approved by michaelwoerister

@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 8, 2019
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Feb 10, 2019
Closed
@bors
Copy link
Contributor

bors commented Feb 12, 2019

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

@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 Feb 12, 2019
@nnethercote
Reduce the size of hir::Expr.
5d65e8c 
From 104 bytes to 72 bytes on x86-64. This slightly reduces instruction
counts.

Also add an assertion about the size.
@nnethercote
Copy link
Contributor Author

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Feb 14, 2019

📌 Commit 5d65e8c has been approved by michaelwoerister

@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 Feb 14, 2019
Centril added a commit to Centril/rust that referenced this pull request Feb 14, 2019
@Centril
Rollup merge of rust-lang#58258 - nnethercote:shrink-hir-Expr, r=mich…
f7b35be 
…aelwoerister

Reduce the size of `hir::Expr`.

From 104 bytes to 72 bytes on x86-64. This slightly reduces instruction
counts.

Also add an assertion about the size.
@Centril Centril mentioned this pull request Feb 14, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request Feb 14, 2019
@Centril
Rollup merge of rust-lang#58258 - nnethercote:shrink-hir-Expr, r=mich…
b6ed206 
…aelwoerister

Reduce the size of `hir::Expr`.

From 104 bytes to 72 bytes on x86-64. This slightly reduces instruction
counts.

Also add an assertion about the size.
@Centril Centril mentioned this pull request Feb 14, 2019
Merged
bors added a commit that referenced this pull request Feb 14, 2019
@bors
Auto merge of #58446 - Centril:rollup, r=Centril
c67d474 
Rollup of 8 pull requests

Successful merges:

 - #57451 (suggestion-diagnostics: as_ref improve snippet)
 - #57856 (Convert old first edition links to current edition one)
 - #57992 (Update the future/task API)
 - #58258 (Reduce the size of `hir::Expr`.)
 - #58267 (Tweak "incompatible match arms" error)
 - #58296 (Hidden suggestion support)
 - #58301 (Enable comparing fat pointers)
 - #58308 (Extract block to insert an intrinsic into its own function)

Failed merges:

r? @ghost
@bors bors merged commit 5d65e8c into rust-lang:master Feb 14, 2019
@nnethercote nnethercote deleted the shrink-hir-Expr branch February 15, 2019 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nnethercote @rust-highfive @bors @michaelwoerister @rust-timer