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

Use `Lit` rather than `P<Lit>` in `ast::ExprKind`. #55777

Merged
merged 1 commit into from Nov 9, 2018

Conversation

Projects
None yet
5 participants
@nnethercote
Contributor

nnethercote commented Nov 8, 2018

Because it results in fewer allocations and small speedups on some
benchmarks.

Use `Lit` rather than `P<Lit>` in `ast::ExprKind`.
Because it results in fewer allocations and small speedups on some
benchmarks.
@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Nov 8, 2018

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Nov 8, 2018

What's the size of ast::Expr before and after on 64 and 32 bit targets?

@nnethercote

This comment has been minimized.

Contributor

nnethercote commented Nov 8, 2018

I checked: the size is unchanged. I don't have the exact numbers on hand, but the Lit variant is far from the largest even after the change.

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Nov 8, 2018

Great.
@bors r+

@bors

This comment has been minimized.

Contributor

bors commented Nov 8, 2018

📌 Commit 706c2ad has been approved by petrochenkov

@nnethercote

This comment has been minimized.

Contributor

nnethercote commented Nov 8, 2018

I have the numbers now: on 64-bit, ExprKind is 72 bytes. The Lit variant was 15 bytes, it's now 39 bytes. There are 6 variants that are larger than that, and another 4 that are also 39 bytes.

@Mark-Simulacrum

This comment has been minimized.

Member

Mark-Simulacrum commented Nov 9, 2018

@bors rollup

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Nov 9, 2018

Rollup merge of rust-lang#55777 - nnethercote:less-P-in-ast, r=petroc…
…henkov

Use `Lit` rather than `P<Lit>` in `ast::ExprKind`.

Because it results in fewer allocations and small speedups on some
benchmarks.

bors added a commit that referenced this pull request Nov 9, 2018

Auto merge of #55803 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
Rollup of 17 pull requests

Successful merges:

 - #55576 (Clarify error message for -C opt-level)
 - #55633 (Support memcpy/memmove with differing src/dst alignment)
 - #55638 (Fix ICE in msg_span_from_free_region on ReEmpty)
 - #55659 (rustc: Delete grouping logic from the musl target)
 - #55719 (Sidestep link error from rustfix'ed code by using a *defined* static.)
 - #55736 (Elide anon lifetimes in conflicting impl note)
 - #55739 (Consume optimization fuel from the MIR inliner)
 - #55742 (Avoid panic when matching function call)
 - #55753 (borrow_set: remove a helper function and a clone it uses)
 - #55755 (Improve creation of 3 IndexVecs)
 - #55758 ([regression - rust2018]: unused_mut lint false positives on nightly)
 - #55760 (Remove intermediate font specs)
 - #55761 (mir: remove a hacky recursive helper function)
 - #55774 (wasm32-unknown-emscripten expects the rust_eh_personality symbol)
 - #55777 (Use `Lit` rather than `P<Lit>` in `ast::ExprKind`.)
 - #55783 (Deprecate mpsc channel selection)
 - #55788 (rustc: Request ansi colors if stderr isn't a tty)

Failed merges:

r? @ghost

@bors bors merged commit 706c2ad into rust-lang:master Nov 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nnethercote nnethercote deleted the nnethercote:less-P-in-ast branch Nov 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment