Rustdoc: use is_doc_hidden method on more places
#92227
Conversation
rustbot
added
the
T-rustdoc
rust-highfive
added
the
S-waiting-on-review
|
I'm on vacation. r? rust-lang/rustdoc |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
rustbot
added
the
S-waiting-on-perf
|
⌛ Trying commit 0ec3199 with merge 7ea1327e5dec6e8f28c44c1c1da52418eec8055e... |
|
☀️ Try build successful - checks-actions |
|
Queued 7ea1327e5dec6e8f28c44c1c1da52418eec8055e with parent 489296d, future comparison URL. |
|
Finished benchmarking commit (7ea1327e5dec6e8f28c44c1c1da52418eec8055e): comparison url. Summary: This change led to very large relevant improvements 🎉 in compiler performance.
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
removed
the
S-waiting-on-perf
|
Wow, that seems nice. It could have been caused by missed inlining, because before it was going through a trait, and it was probably making some additional copies. I wonder if it's even worth it to cache the result now. But I'm still not if this is correct :) |
jyn514
added
the
I-compiletime
The compiler already caches query executions, please don't add more layers on top. |
|
But |
|
Yes, you're right, it's just a regular method. I would leave it as is for now, it doesn't seem particularly expensive to calculate. If you're interested in making it a query that can go in a new PR. |
|
@bors r+ Great find! |
|
📌 Commit 0ec3199 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (c096176): comparison url. Summary: This change led to very large relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
…Gomez Add Attribute::meta_kind The `AttrItem::meta` function is being called on a lot of places, however almost always the caller is only interested in the `kind` of the result `MetaItem`. Before, the `path` had to be cloned in order to get the kind, now it does not have to be. There is a larger related "problem". In a lot of places, something wants to know contents of attributes. This is accessed through `Attribute::meta_item_list`, which calls `AttrItem::meta` (now `AttrItem::meta_kind`), among other methods. When this function is called, the meta item list has to be recreated from scratch. Everytime something asks a simple question (like is this item/list of attributes `#[doc(hidden)]`?), the tokens of the attribute(s) are cloned, parsed and the results are allocated on the heap. That seems really unnecessary. What would be the best way to cache this? Turn `meta_item_list` into a query perhaps? Related PR: rust-lang#92227 r? rust-lang/rustdoc
…eGomez Rustdoc: remove ListAttributesIter and use impl Iterator instead This is a continuation of rust-lang#92227. I found that `ListAttributesIter` did not optimize well and replacing it with a simple `impl Iterator` resulted in 1-3 % instruction count wins locally. Because I needed to use `impl Iterator` on a slice of AST attributes, I had to implement it using GAT + impl trait. I also have a version without GAT [here](Kobzol@5470e2a), if GATs are not welcome in rustdoc :D Locally it resulted in equal performance numbers. Can I ask for a perf. run? Thanks. r? rust-lang/rustdoc
While profiling
rustdoc, I noticed that finding out if some item is marked with#[doc(hidden)]is relatively hot, so I tried to optimize it.I noticed that there is already a method called
is_doc_hiddenonTyCtxt, but it wasn't used much, so I replaced the manual calls toattrs(...).has_word(...)with this method. Just by doing that, perf. was improved locally, although I'm not sure if the semantics of the previous calls and this method are the same?As another step, I tried to querify
is_doc_hidden, but I didn't include that here until we see the perf. results from the first commit and until I find whether this change is OK at all :)Can I ask for a perf. run? Thanks.
r? @jyn514