Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add rustdoc performance blog post #760

Merged
merged 6 commits into from
Jan 20, 2021
Merged

Add rustdoc performance blog post #760

merged 6 commits into from
Jan 20, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jan 15, 2021

@jyn514 jyn514 added needs review content An issue related to an article. labels Jan 15, 2021
@jyn514 jyn514 requested a review from a team as a code owner January 15, 2021 19:43
@GuillaumeGomez
Copy link
Member

👍

@jyn514
Copy link
Member Author

jyn514 commented Jan 15, 2021

Oh hold on - @bugadani mentioned that we should probably mention the actual performance PRs, which are caching intra-doc links and not looking for blanket impls. Let's hold off on merging until then.

@jyn514
Review comments
b6e38f8 
- Add PRs targeted at intra-doc link performance
- Be more environmentally friendly
@jyn514
Copy link
Member Author

jyn514 commented Jan 15, 2021

Ok, this is ready for review.

Manishearth
Manishearth approved these changes Jan 16, 2021
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, might need someone to go through and make copy edits

posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
jyn514 and others added 2 commits January 15, 2021 20:37
@jyn514 @Manishearth
Improve wording
4f7ad31 
Co-authored-by: Manish Goregaokar <[email protected]>
@jyn514 @bugadani
Fix hyperlink
f11dd00 
Co-authored-by: Dániel Buga <[email protected]>
GuillaumeGomez
GuillaumeGomez reviewed Jan 16, 2021
View reviewed changes
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md

## Errata

An earlier version of the blog post described the section on slimming `doctree` as "Burning down
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes me sad. It was a funny pun. :'(

camelid
camelid reviewed Jan 16, 2021
View reviewed changes
Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great blog post! I have some comments that are mostly typographical nits and also a typo or two :)

Some of them are stylistic choices, so they're not a big deal, but I wanted to suggest possible alternatives to see what you think.

posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Outdated Show resolved Hide resolved
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md Show resolved Hide resolved
@jyn514
Copy link
Member Author

jyn514 commented Jan 17, 2021

@camelid I've given you write access to my fork, feel free to make all the edits you want. I'll try to address the review comments at some point but I may not get around to it today.

@GuillaumeGomez GuillaumeGomez merged commit a387081 into rust-lang:master Jan 20, 2021
@jyn514 jyn514 deleted the rustdoc branch January 20, 2021 15:54
bugadani
bugadani reviewed Jan 22, 2021
View reviewed changes
posts/inside-rust/2021-01-15-rustdoc-performance-improvements.md
1. Rustdoc: Cache resolved links [#77700](https://github.com/rust-lang/rust/pull/77700)

This does what it says in the title. In particular, this sped up the time to generate intra-doc
links for `stm32h7xx` by a whopping [90,000%]. [**@bugadani**](https://github.com/bugadani) did an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized, 90x =/= 90000%. I think this is a zero off :)

Copy link
Member

@camelid camelid Jan 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment this links to seems to show an 82x speedup. Maybe we should just say "by a whopping 82x"? Or "this was an 82x speedup"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a huge percentage looks alot better tho 🤓

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

@camelid camelid camelid left review comments

@tesuji tesuji tesuji left review comments

@therealprof therealprof therealprof left review comments

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

@bugadani bugadani bugadani left review comments

@Manishearth Manishearth Manishearth approved these changes

Assignees
No one assigned
Labels
content An issue related to an article. needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jyn514 @GuillaumeGomez @bugadani @Manishearth @therealprof @tesuji @camelid