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.

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

Avoid unnecessary arena allocations in expand_pattern(). #65463

Merged

Conversation

nnethercote
Copy link
Contributor

expand_pattern() has two callsites. One of them needs arena
allocation, but the other does not.

This commit moves the arena allocation out of the function. This avoids
the allocation of many 4 KiB page arena chunks that only hold a single
small allocation. It reduces the number of bytes allocated by up to 2%
for various benchmarks, albeit without only a very small improvement in
runtime.

`expand_pattern()` has two callsites. One of them needs arena
allocation, but the other does not.

This commit moves the arena allocation out of the function. This avoids
the allocation of many 4 KiB page arena chunks that only hold a single
small allocation. It reduces the number of bytes allocated by up to 2%
for various benchmarks, albeit without only a very small improvement in
runtime.
@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 Oct 16, 2019
@nnethercote
Copy link
Contributor Author

r? @varkor

@rust-highfive rust-highfive assigned varkor and unassigned petrochenkov Oct 16, 2019
@Centril
Copy link
Contributor

Centril commented Oct 16, 2019

Let's block this on landing #65160?

@varkor
Copy link
Member

varkor commented Oct 20, 2019

#65160 is going to have to be rebased anyway, and this is a small change, so I think merging this now should be fine.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 20, 2019

📌 Commit c4deea2 has been approved by varkor

@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 Oct 20, 2019
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 20, 2019
…m-expand_pattern, r=varkor

Avoid unnecessary arena allocations in `expand_pattern()`.

`expand_pattern()` has two callsites. One of them needs arena
allocation, but the other does not.

This commit moves the arena allocation out of the function. This avoids
the allocation of many 4 KiB page arena chunks that only hold a single
small allocation. It reduces the number of bytes allocated by up to 2%
for various benchmarks, albeit without only a very small improvement in
runtime.
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 20, 2019
…m-expand_pattern, r=varkor

Avoid unnecessary arena allocations in `expand_pattern()`.

`expand_pattern()` has two callsites. One of them needs arena
allocation, but the other does not.

This commit moves the arena allocation out of the function. This avoids
the allocation of many 4 KiB page arena chunks that only hold a single
small allocation. It reduces the number of bytes allocated by up to 2%
for various benchmarks, albeit without only a very small improvement in
runtime.
bors added a commit that referenced this pull request Oct 20, 2019
Rollup of 5 pull requests

Successful merges:

 - #65460 (Clean up `contains()` `insert()` chains on HashSet)
 - #65463 (Avoid unnecessary arena allocations in `expand_pattern()`.)
 - #65579 (Changed `resolve_type_vars_with_obligations` to also resolve const inference variables)
 - #65605 (Remove unreachable unit tuple compare binop codegen)
 - #65626 (trivial typo fix)

Failed merges:

r? @ghost
@bors bors merged commit c4deea2 into rust-lang:master Oct 20, 2019
@nnethercote nnethercote deleted the rm-arena-allocation-from-expand_pattern branch October 21, 2019 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants