Yet more `parse_tt` improvements #95425

nnethercote · 2022-03-29T06:45:02Z

Including lots of comment improvements, and an overhaul of how matches work that gives big speedups.

nnethercote · 2022-03-29T06:47:38Z

Some check full instruction count results.

Benchmark & Profile	Scenario	% Change	Significance Factor?
quote-stress check	full	-17.38%	86.90x
async-std-1.10.0 check	full	-16.64%	83.18x
time-macros-0.2.3 check	full	-10.55%	52.74x
yansi-0.5.0 check	full	-9.62%	48.10x
deep-vector check	full	-2.85%	14.26x
num-derive-0.3.3 check	full	-2.51%	12.56x
pest_generator-2.1.3 check	full	-2.45%	12.27x
inotify-0.10.0 check	full	-2.30%	11.49x
scroll_derive-0.11.0 check	full	-2.28%	11.41x
ctor-0.1.21 check	full	-2.25%	11.26x
mockall_derive-0.11.0 check	full	-1.98%	9.90x
futures-macro-0.3.19 check	full	-1.89%	9.46x
html5ever check	full	-0.85%	4.27x
token-stream-stress check	full	0.66%	3.32x
syn-1.0.89 check	full	-0.48%	2.38x
hyper-0.14.18 check	full	-0.41%	2.07x

nnethercote · 2022-03-29T06:47:54Z

@bors try @rust-timer queue

rust-timer · 2022-03-29T06:47:55Z

Awaiting bors try build completion.

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

bors · 2022-03-29T06:48:02Z

⌛ Trying commit 89325098160f8ee473b0ebfc1b05c323254a1f19 with merge 87e9e2c9089a81be4a5b32d55832388b56a3e885...

bors · 2022-03-29T08:08:55Z

☀️ Try build successful - checks-actions
Build commit: 87e9e2c9089a81be4a5b32d55832388b56a3e885 (87e9e2c9089a81be4a5b32d55832388b56a3e885)

rust-timer · 2022-03-29T08:08:56Z

Queued 87e9e2c9089a81be4a5b32d55832388b56a3e885 with parent ad5b6f6, future comparison URL.

compiler/rustc_expand/src/mbe/macro_parser.rs

petrochenkov · 2022-03-29T12:23:57Z

compiler/rustc_expand/src/mbe/macro_parser.rs

-                // != Meta-variable expressions`
+                // RHS meta-variable expressions eventually end-up here. `0` is returned to inform that
+                // no meta-variable was found, because "meta-variables" != "meta-variable
+                // expressions".
                TokenTree::MetaVarExpr(..) => 0,


This should be unreachable, and count_metavars should never be called when parsing RHS (parsing_patterns == false) in the first place, it's 0 there by definition.

I originally thought that too, but if you make it unreachable these tests fail:

failures: [ui] ui/macros/rfc-3086-metavar-expr/count-and-length-are-distinct.rs [ui] ui/macros/rfc-3086-metavar-expr/feature-gate-macro_metavar_expr.rs [ui] ui/macros/rfc-3086-metavar-expr/macro-expansion.rs [ui] ui/macros/rfc-3086-metavar-expr/out-of-bounds-arguments.rs

There may be something to clean up here, but I'd prefer to do it in a follow-up. This change was just meant to give this function a nicer name and slightly cleaner code, it wasn't doing anything about where/how it is called.

compiler/rustc_expand/src/mbe/macro_parser.rs

rust-timer · 2022-03-29T15:43:23Z

Finished benchmarking commit (87e9e2c9089a81be4a5b32d55832388b56a3e885): comparison url.

Summary: This benchmark run shows 45 relevant improvements 🎉 to instruction counts.

Arithmetic mean of relevant improvements: -1.4%
Arithmetic mean of all relevant changes: -1.4%
Largest improvement in instruction counts: -6.2% on incr-unchanged builds of deep-vector check

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. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf.

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

In particular: - Replace use of "item" with "matcher position/"mp". - Replace use of "repetition" with "sequence". - Replace `ms` with `matcher`.

Currently, matches within a sequence are recorded in a new empty `matches` vector. Then when the sequence finishes the matches are merged into the `matches` vector of the parent. This commit changes things so that a sequence mp inherits the matches made so far. This means that additional matches from the sequence don't need to be merged into the parent. `push_match` becomes more complicated, and the current sequence depth needs to be tracked. But it's a sizeable performance win because it avoids one or more `push_match` calls on every iteration of a sequence. The commit also removes `match_hi`, which is no longer necessary.

This avoids some allocations.

nnethercote · 2022-03-30T00:12:33Z

I have updated the code to address comments. Most of the changes are in the relevant commits, but I split out the empty_matches change into a new commit.

petrochenkov · 2022-03-30T11:36:18Z

@bors r+

bors · 2022-03-30T11:36:20Z

📌 Commit 6b0a16a has been approved by petrochenkov

bors · 2022-03-30T19:08:04Z

⌛ Testing commit 6b0a16a with merge c5cf08d...

bors · 2022-03-30T21:34:21Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing c5cf08d to master...

rust-timer · 2022-03-31T00:00:28Z

Finished benchmarking commit (c5cf08d): comparison url.

Summary: This benchmark run shows 53 relevant improvements 🎉 to instruction counts.

Arithmetic mean of relevant improvements: -1.4%
Arithmetic mean of all relevant changes: -1.4%
Largest improvement in instruction counts: -6.2% on incr-unchanged builds of deep-vector check

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

@rustbot label: -perf-regression

expand: Do not count metavar declarations on RHS of `macro_rules` They are 0 by definition there. Addresses rust-lang#95425 (comment) r? `@nnethercote`

expand: Do not count metavar declarations on RHS of `macro_rules` They are 0 by definition there. Addresses rust-lang#95425 (comment) r? ``@nnethercote``

expand: Do not count metavar declarations on RHS of `macro_rules` They are 0 by definition there. Addresses rust-lang#95425 (comment) r? ```@nnethercote```

Add a useful assertion.

df6ead5

rust-highfive assigned petrochenkov Mar 29, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 29, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 29, 2022

petrochenkov reviewed Mar 29, 2022

View reviewed changes

petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2022

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 Mar 29, 2022

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2022

nnethercote added 5 commits March 30, 2022 10:42

Simplify and rename count_names.

2b60cc0

Clarify comments about doc comments in macros.

ac3d8ce

Improve comments and rename many things for consistency.

a1b140c

In particular: - Replace use of "item" with "matcher position/"mp". - Replace use of "repetition" with "sequence". - Replace `ms` with `matcher`.

Pre-allocate an empty Lrc<NamedMatchVec>.

6b0a16a

This avoids some allocations.

nnethercote force-pushed the yet-more-parse_tt-improvements branch from 8932509 to 6b0a16a Compare March 30, 2022 00:12

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 30, 2022

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 Mar 30, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2022

bors merged commit c5cf08d into rust-lang:master Mar 30, 2022

rustbot added this to the 1.61.0 milestone Mar 30, 2022

nnethercote deleted the yet-more-parse_tt-improvements branch March 31, 2022 02:08

petrochenkov mentioned this pull request Mar 31, 2022

expand: Do not count metavar declarations on RHS of macro_rules #95531

Merged

lqd mentioned this pull request Sep 15, 2022

internal compiler error: opaque type with non-universal region substs #101852

Closed

matthiaskrgr mentioned this pull request Apr 20, 2023

ICE: "no errors encountered even though delay_span_bug issued" in recursive impl #110623

Closed

lqd mentioned this pull request Jun 8, 2023

unchecked region constraints for opaque types in dead code #112417

Open

lqd mentioned this pull request Aug 11, 2023

RPIT hidden types can be ill-formed #114728

Closed

This was referenced Mar 28, 2024

ICE: {OpaqueTypeKey { def_id: DefId .. #123157

Open

ICE: None in compiler/rustc_mir_build/src/build/matches/mod.rs #119786

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Yet more `parse_tt` improvements #95425

Yet more `parse_tt` improvements #95425

nnethercote commented Mar 29, 2022

nnethercote commented Mar 29, 2022

nnethercote commented Mar 29, 2022

rust-timer commented Mar 29, 2022

bors commented Mar 29, 2022

bors commented Mar 29, 2022

rust-timer commented Mar 29, 2022

petrochenkov Mar 29, 2022 •

edited

Loading

nnethercote Mar 29, 2022

petrochenkov Mar 30, 2022

rust-timer commented Mar 29, 2022

nnethercote commented Mar 30, 2022

petrochenkov commented Mar 30, 2022

bors commented Mar 30, 2022

bors commented Mar 30, 2022

bors commented Mar 30, 2022

rust-timer commented Mar 31, 2022

Yet more parse_tt improvements #95425

Yet more parse_tt improvements #95425

Conversation

nnethercote commented Mar 29, 2022

nnethercote commented Mar 29, 2022

nnethercote commented Mar 29, 2022

rust-timer commented Mar 29, 2022

bors commented Mar 29, 2022

bors commented Mar 29, 2022

rust-timer commented Mar 29, 2022

petrochenkov Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

nnethercote Mar 29, 2022

Choose a reason for hiding this comment

petrochenkov Mar 30, 2022

Choose a reason for hiding this comment

rust-timer commented Mar 29, 2022

nnethercote commented Mar 30, 2022

petrochenkov commented Mar 30, 2022

bors commented Mar 30, 2022

bors commented Mar 30, 2022

bors commented Mar 30, 2022

rust-timer commented Mar 31, 2022

Yet more `parse_tt` improvements #95425

Yet more `parse_tt` improvements #95425

petrochenkov Mar 29, 2022 •

edited

Loading