Skip to content

Use more symbols in rustdoc - #80047

Merged
bors merged 2 commits into
rust-lang:masterfrom
jyn514:more-symbols
Dec 17, 2020
Merged

Use more symbols in rustdoc#80047
bors merged 2 commits into
rust-lang:masterfrom
jyn514:more-symbols

Conversation

@jyn514

@jyn514jyn514 commented Dec 15, 2020

Copy link
Copy Markdown
Member

Builds on #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

@jyn514jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 15, 2020
@jyn514
Remove redundant assignment
c18c7c7
`crate.name` is already set by `tcx.crate_name`, there's no need to
override it.
@jyn514
Use Symbols for crate names
7ee8e18
@jyn514

jyn514 commented Dec 16, 2020

Copy link
Copy Markdown
MemberAuthor

@bors try @rust-timer queue

@rust-timer

rust-timer commented Dec 16, 2020

Copy link
Copy Markdown
Collaborator

Awaiting bors try build completion.

@bors

bors commented Dec 16, 2020

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 7ee8e18 with merge af8283101a3e4aa82fbf6b6a660fe105a85c9297...

@bors

bors commented Dec 16, 2020

Copy link
Copy Markdown
Collaborator

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

@rust-timer

rust-timer commented Dec 16, 2020

Copy link
Copy Markdown
Collaborator

Queued af8283101a3e4aa82fbf6b6a660fe105a85c9297 with parent 4031f7b, future comparison URL.

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

@rustbotrustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 16, 2020
@jyn514jyn514 removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Dec 16, 2020
@rust-timer

rust-timer commented Dec 16, 2020

Copy link
Copy Markdown
Collaborator

Finished benchmarking try commit (af8283101a3e4aa82fbf6b6a660fe105a85c9297): 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 label: +S-waiting-on-review -S-waiting-on-perf

@rustbotrustbot 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 Dec 16, 2020
@jyn514

jyn514 commented Dec 16, 2020

Copy link
Copy Markdown
MemberAuthor

Both instruction counts and max-rss look like noise. @GuillaumeGomez said he was working on converting even more strings to symbols, maybe that will combine well? I don't feel super strongly about whether this lands.

@GuillaumeGomezGuillaumeGomez mentioned this pull request Dec 16, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 17, 2020
@bors
Auto merge ofrust-lang#80091- GuillaumeGomez:str-to-symbol, r=jyn514
bdd0a78
Replace String with Symbol where possible
The same as rust-lang#80047 but on different types. Might be interesting to run some perf comparison.
r? `@jyn514`
@GuillaumeGomez

GuillaumeGomez commented Dec 17, 2020

Copy link
Copy Markdown
Member

Let's get this in as well! Thanks!

@bors: r+ rollup

@bors

bors commented Dec 17, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 7ee8e18 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 17, 2020
@GuillaumeGomezGuillaumeGomez mentioned this pull request Dec 17, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 17, 2020
@bors
Auto merge ofrust-lang#80114- GuillaumeGomez:rollup-gszr5kn, r=Guil…
caeb333
…laumeGomez
Rollup of 5 pull requests
Successful merges:
- rust-lang#80006 (BTreeMap: more expressive local variables in merge)
- rust-lang#80022 (BTreeSet: simplify implementation of pop_first/pop_last)
- rust-lang#80035 (Optimization for bool's PartialOrd impl)
- rust-lang#80040 (Always run intrinsics lowering pass)
- rust-lang#80047 (Use more symbols in rustdoc)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@jyn514

jyn514 commented Dec 17, 2020

Copy link
Copy Markdown
MemberAuthor

@GuillaumeGomez anything that affects perf should not be rolled up, it means you can't find which PRs had an impact and it mask regressions.

@bors
bors merged commit 5873fe8 into rust-lang:masterDec 17, 2020
@GuillaumeGomez

GuillaumeGomez commented Dec 17, 2020

Copy link
Copy Markdown
Member

Arf indeed... The rollup=never got overwritten I guess. Not great and bad PR handling on my side... :-/

@rustbotrustbot added this to the 1.50.0 milestone Dec 17, 2020
@jyn514
jyn514 deleted the more-symbols branch December 17, 2020 15:03
@jyn514

jyn514 commented Dec 17, 2020

Copy link
Copy Markdown
MemberAuthor

It's ok, fortunately this one didn't have much of an impact.

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

Reviewers

No reviews

Assignees

@GuillaumeGomezGuillaumeGomez

Labels

C-cleanupCategory: PRs that clean code up or issues documenting cleanup.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.

5 participants

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