Cheaper doc comments#65750

nnethercote · 2019-10-24T03:28:35Z

This PR implements the idea from #60935: represent doc comments more cheaply, rather than converting them into #[doc="..."] attribute form. Unlike #60936 (which is about coalescing doc comments to reduce their number), this approach does not have any backwards compatibility concerns, and it eliminates about 80-90% of the current cost of doc comments (as estimated using the numbers in #60930, which eliminated the cost of doc comments entirely by treating them as normal comments).

r? @petrochenkov

nnethercote · 2019-10-24T03:29:51Z

@bors try @rust-timer queue

rust-timer · 2019-10-24T03:29:53Z

Awaiting bors try build completion

bors · 2019-10-24T03:30:04Z

⌛ Trying commit 0b907f7ed55cd4b79ccb380e68fcca0dacc0f0e2 with merge b773dddc5a6434753e502b8beb1868fd476103f8...

nnethercote · 2019-10-24T03:48:55Z

@Manishearth, @nrc: this PR will affect Clippy and rustfmt.

nnethercote · 2019-10-24T19:37:44Z

@rust-timer build b773dddc5a6434753e502b8beb1868fd476103f8

rust-timer · 2019-10-24T19:37:47Z

Queued b773dddc5a6434753e502b8beb1868fd476103f8 with parent 4a8c5b2, future comparison URL.

rust-timer · 2019-10-25T03:00:30Z

Finished benchmarking try commit b773dddc5a6434753e502b8beb1868fd476103f8, comparison URL.

nnethercote · 2019-10-25T04:00:27Z

Perf results are excellent, as expected.

Centril · 2019-10-25T04:53:02Z

r? @Centril @bors r+

bors · 2019-10-25T04:53:03Z

📌 Commit 42bac153bb9e9baaf30f9aaa2830faf69042ac99 has been approved by Centril

Centril · 2019-10-25T04:55:48Z

@bors rollup=never (perf)

petrochenkov · 2019-10-25T05:15:06Z

@bors r-
I really wanted to review this, can it wait for a day or so?

Centril · 2019-10-25T05:17:49Z

Sure -- I don't mind.
(I did double check the diff and it all seems good.)

nrc · 2019-10-25T08:41:28Z

cc @topecongiro for possible rustfmt breakage

bors · 2019-10-25T14:37:05Z

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

nnethercote · 2019-10-25T19:28:22Z

I have rebased.

This kind of thing just makes the code harder to read.

`AttrKind` is a new type with two variants, `Normal` and `DocComment`. It's a big performance win (over 10% in some cases) because `DocComment` lets doc comments (which are common) be represented very cheaply. `Attribute` gets some new helper methods to ease the transition: - `has_name()`: check if the attribute name matches a single `Symbol`; for `DocComment` variants it succeeds if the symbol is `sym::doc`. - `is_doc_comment()`: check if it has a `DocComment` kind. - `{get,unwrap}_normal_item()`: extract the item from a `Normal` variant; panic otherwise. Fixes rust-lang#60935.

nnethercote · 2019-11-06T12:05:49Z

@bors r=petrochenkov

bors · 2019-11-06T12:05:50Z

📌 Commit eea6f23 has been approved by petrochenkov

bors · 2019-11-07T00:10:59Z

⌛ Testing commit eea6f23 with merge caf0187...

@petrochenkov

Cheaper doc comments This PR implements the idea from #60935: represent doc comments more cheaply, rather than converting them into `#[doc="..."]` attribute form. Unlike #60936 (which is about coalescing doc comments to reduce their number), this approach does not have any backwards compatibility concerns, and it eliminates about 80-90% of the current cost of doc comments (as estimated using the numbers in #60930, which eliminated the cost of doc comments entirely by treating them as normal comments). r? @petrochenkov

bors · 2019-11-07T03:54:17Z

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing caf0187 to master...

tesuji · 2019-11-12T15:02:23Z

The final result: https://perf.rust-lang.org/compare.html?start=38048763e885a3ee139abf39d59a530b16484150&end=caf018714189db0b15f9f803adfcb4572ab7a988&stat=instructions%3Au

@nnethercote

syntax: Unify macro and attribute arguments in AST The unified form (`ast::MacArgs`) represents parsed arguments instead of an unstructured token stream that was previously used for attributes. It also tracks some spans and delimiter kinds better for fn-like macros and macro definitions. I've been talking about implementing this with @nnethercote in rust-lang#65750 (comment). The parsed representation is closer to `MetaItem` and requires less token juggling during conversions, so it potentially may be faster. r? @Centril

@nnethercote

syntax: Unify macro and attribute arguments in AST The unified form (`ast::MacArgs`) represents parsed arguments instead of an unstructured token stream that was previously used for attributes. It also tracks some spans and delimiter kinds better for fn-like macros and macro definitions. I've been talking about implementing this with @nnethercote in rust-lang#65750 (comment). The parsed representation is closer to `MetaItem` and requires less token juggling during conversions, so it potentially may be faster. r? @Centril

doc comments: Less attribute mimicking Make sure doc comments are not converted into intermediate meta-items, or not mixed with `doc(inline)` or something like that. Follow-up to rust-lang#65750.

doc comments: Less attribute mimicking Make sure doc comments are not converted into intermediate meta-items, or not mixed with `doc(inline)` or something like that. Follow-up to #65750.

Implements changes required post rust-lang/rust#65750

rust-highfive assigned petrochenkov Oct 24, 2019

This was referenced Oct 24, 2019

Coalescing for line doc comments #60936

Closed

Do not convert doc comments into #[doc = "..."] form unless necessary for macros #60935

Closed

nnethercote force-pushed the cheaper-doc-comments branch from 0b907f7 to 42bac15 Compare October 24, 2019 05:31

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2019

rust-highfive assigned Centril and unassigned petrochenkov Oct 25, 2019

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 25, 2019

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 Oct 25, 2019

petrochenkov self-assigned this Oct 25, 2019

nnethercote force-pushed the cheaper-doc-comments branch from 42bac15 to 7afba1a Compare October 25, 2019 19:28

nnethercote added 2 commits November 6, 2019 23:01

Remove unnecessary Deref impl for Attribute.

69bc4ab

This kind of thing just makes the code harder to read.

nnethercote force-pushed the cheaper-doc-comments branch from 9964212 to eea6f23 Compare November 6, 2019 12:05

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 Nov 6, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 7, 2019

bors merged commit eea6f23 into rust-lang:master Nov 7, 2019

This was referenced Nov 7, 2019

syntax: move stuff around #65916

Merged

syntax: ABI-oblivious grammar #65884

Merged

msizanoen1 added a commit to msizanoen1/rust-clippy that referenced this pull request Nov 7, 2019

Fix libsyntax breaking changes caused by rust-lang/rust#65750

e12c00d

therealprof mentioned this pull request Nov 12, 2019

Is there any way to create inline comments? dtolnay/proc-macro2#205

Closed

nnethercote deleted the cheaper-doc-comments branch November 13, 2019 21:53

petrochenkov mentioned this pull request Dec 1, 2019

syntax: Unify macro and attribute arguments in AST #66935

Merged

petrochenkov mentioned this pull request Dec 8, 2019

doc comments: Less attribute mimicking #67151

Merged

calebcartwright added a commit to calebcartwright/rustfmt that referenced this pull request Jan 16, 2020

refactor: apply AST doc comment updates

30ed45b

Implements changes required post rust-lang/rust#65750

Mark-Simulacrum mentioned this pull request Apr 6, 2020

doc attribute removes whitespace #70732

Closed

nnethercote mentioned this pull request Aug 3, 2020

rustc_metadata: track the simplified Self type for every trait impl. #75008

Merged

Uh oh!

Conversation

nnethercote commented Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Oct 24, 2019

Uh oh!

rust-timer commented Oct 24, 2019

Uh oh!

bors commented Oct 24, 2019

Uh oh!

nnethercote commented Oct 24, 2019

Uh oh!

nnethercote commented Oct 24, 2019

Uh oh!

rust-timer commented Oct 24, 2019

Uh oh!

rust-timer commented Oct 25, 2019

Uh oh!

nnethercote commented Oct 25, 2019

Uh oh!

Centril commented Oct 25, 2019

Uh oh!

bors commented Oct 25, 2019

Uh oh!

Centril commented Oct 25, 2019

Uh oh!

petrochenkov commented Oct 25, 2019

Uh oh!

Centril commented Oct 25, 2019

Uh oh!

nrc commented Oct 25, 2019

Uh oh!

bors commented Oct 25, 2019

Uh oh!

nnethercote commented Oct 25, 2019

Uh oh!

nnethercote commented Nov 6, 2019

Uh oh!

bors commented Nov 6, 2019

Uh oh!

bors commented Nov 7, 2019

Uh oh!

bors commented Nov 7, 2019

Uh oh!

tesuji commented Nov 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nnethercote commented Oct 24, 2019 •

edited

Loading