Skip to content

[rustdoc] Switch to Symbol for item.name - #80044

Merged
bors merged 2 commits into
rust-lang:masterfrom
jyn514:smaller-name
Dec 15, 2020
Merged

[rustdoc] Switch to Symbol for item.name#80044
bors merged 2 commits into
rust-lang:masterfrom
jyn514:smaller-name

Conversation

@jyn514

@jyn514jyn514 commented Dec 15, 2020•
edited
Loading

Copy link
Copy Markdown
Member

This decreases the size of Item from 680 to 616 bytes. It also does a
lot less work since it no longer has to copy as much.

Helps with #79103.

r? @GuillaumeGomez

@jyn514
Remove unnecessary unwrap_or
89fc503
This was always questionable, and removing it doesn't fail any tests, so
I think this was not affecting the behavior. It dates all the way back
to the very first commit of rustdoc: 268f3f0
@jyn514jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Dec 15, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 15, 2020
@jyn514
Switch to Symbol for item.name
a16904f
This decreases the size of `Item` from 680 to 616 bytes. It also does a
lot less work since it no longer has to copy as much.
@jyn514

jyn514 commented Dec 15, 2020

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rust-timer

rust-timer commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion

@bors

bors commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

⌛ Trying commit a16904f with merge 7ca105dfbe1b53e0dfb4c59e00a8a9defd0f7ea5...

@bors

bors commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

☀️ Try build successful - checks-actions
Build commit: 7ca105dfbe1b53e0dfb4c59e00a8a9defd0f7ea5 (7ca105dfbe1b53e0dfb4c59e00a8a9defd0f7ea5)

@rust-timer

rust-timer commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

Queued 7ca105dfbe1b53e0dfb4c59e00a8a9defd0f7ea5 with parent 8b3ee82, future comparison URL.

@jyn514jyn514 mentioned this pull request Dec 15, 2020
Merged
@rust-timer

rust-timer commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

Finished benchmarking try commit (7ca105dfbe1b53e0dfb4c59e00a8a9defd0f7ea5): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-perf

@jyn514

jyn514 commented Dec 15, 2020

Copy link
Copy Markdown
MemberAuthor

Max of -0.5% on instruction count, max of -5% on max-rss. So less than I hoped, but definitely worth landing I think.

@GuillaumeGomez

GuillaumeGomez commented Dec 15, 2020

Copy link
Copy Markdown
Member

Thanks!

@bors: r+

@bors

bors commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit a16904f has been approved by GuillaumeGomez

@borsbors 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 Dec 15, 2020
@bors

bors commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a16904f with merge f76ecd0...

@bors

bors commented Dec 15, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing f76ecd0 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Dec 15, 2020
@bors
bors merged commit f76ecd0 into rust-lang:masterDec 15, 2020
@rustbotrustbot added this to the 1.50.0 milestone Dec 15, 2020
@jyn514
jyn514 deleted the smaller-name branch December 15, 2020 21:44
@borsbors mentioned this pull request Dec 15, 2020
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 17, 2020
@GuillaumeGomez
Rollup merge ofrust-lang#80047- jyn514:more-symbols, r=GuillaumeGomez
5873fe8
Use more symbols in rustdoc
Builds on rust-lang#80044 and should not be merged before.
I want to test if this is actually faster before merging it, there was a lot of `to_string()` calls so I'm not sure it will actually help. That means I have to wait for 80044 to get merged before running perf.
r? `@ghost`
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GuillaumeGomezGuillaumeGomezGuillaumeGomez approved these changes

Assignees

@GuillaumeGomezGuillaumeGomez

Labels

I-compilememIssue: Problems and improvements with respect to memory usage during compilation.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.50.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jyn514@rust-highfive@rust-timer@bors@GuillaumeGomez@rustbot