Stream: rustdoc

Topic: Improving blanket_impls perf


view this post on Zulip Joshua Nelson (Dec 03 2020 at 23:01):

@Caleb Webber are you still planning to work on https://github.com/rust-lang/rust/issues/78761 ? nbd if not but @Poliorcetics was interested in taking a look

view this post on Zulip Poliorcetics (Dec 03 2020 at 23:02):

:wave:

view this post on Zulip Joshua Nelson (Dec 03 2020 at 23:03):

@Poliorcetics also fyi it turns out that blanket impls are completely broken currently due to some bad caching in rustdoc https://github.com/rust-lang/rust/issues/78800

view this post on Zulip Joshua Nelson (Dec 03 2020 at 23:03):

it doesn't return anything if you call get_blanket_impls more than once :face_palm:

view this post on Zulip Joshua Nelson (Dec 03 2020 at 23:04):

@Caleb Webber IIRC the issue with get_blanket_impls was not related to my suggested perf fix, which was 'call blanket_impls less'

view this post on Zulip Joshua Nelson (Dec 03 2020 at 23:04):

so I think it makes sense for you and @Poliorcetics to work on both in parallel

view this post on Zulip Caleb Webber (Dec 04 2020 at 00:41):

(deleted)

view this post on Zulip Caleb Webber (Dec 04 2020 at 00:42):

Joshua Nelson said:

Caleb Webber are you still planning to work on https://github.com/rust-lang/rust/issues/78761 ? nbd if not but Poliorcetics was interested in taking a look

I'm good with this, I'm still working on 78800, haven't really figured out perf issues yet

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:45):

the other thing to consider is whether rustdoc should be using get_blanket_impls at all

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:45):

from what @Caleb Webber was saying it sounds like it's pretty heavily specialized for calculating all impls exactly once

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:45):

which is not what collect_intra_doc_links needs

view this post on Zulip Caleb Webber (Dec 04 2020 at 00:50):

I'd be curious to see what performance impact we'd have by removing that behavior

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:55):

@Manish Goregaokar I pinged you on https://github.com/rust-lang/rust/issues/78800 but you might have missed it - what do you think about just not looking at blanket impls for resolving associated items? It's been broken since implemented so no one can be using it, and it's like a 100x slowdown

view this post on Zulip Caleb Webber (Dec 04 2020 at 00:56):

ha, I'd be okay with that too, less coding :laughing:

view this post on Zulip Manish Goregaokar (Dec 04 2020 at 00:57):

@Joshua Nelson oh I thought I'd responded. Yeah just kill it for now

view this post on Zulip Manish Goregaokar (Dec 04 2020 at 00:57):

I would like to have it if possible

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:57):

yeah I think we could reland it at some point in the future

view this post on Zulip Joshua Nelson (Dec 04 2020 at 00:57):

but as is it's doing nothing and hurting perf :/

view this post on Zulip Caleb Webber (Dec 04 2020 at 01:02):

hmm, are you okay with losing auto traits as well? Not sure how with the current implementation you'd remove blanket impls without losing auto impls

view this post on Zulip Joshua Nelson (Dec 04 2020 at 01:02):

auto traits are just Send and Sync, right? those don't have associated items

view this post on Zulip Caleb Webber (Dec 04 2020 at 01:14):

would this close 78761?

view this post on Zulip Joshua Nelson (Dec 04 2020 at 01:16):

I guess so, yeah. Not the issue that they don't work, though.

view this post on Zulip Caleb Webber (Dec 04 2020 at 01:16):

yeah
I would think we'd want to plan to reimplement something later...

view this post on Zulip Joshua Nelson (Dec 04 2020 at 01:17):

definitely, this is just temporary because it's doing nothing right now

view this post on Zulip Joshua Nelson (Dec 04 2020 at 01:18):

https://github.com/rust-lang/rust/pull/79682

view this post on Zulip Caleb Webber (Dec 04 2020 at 01:21):

ha, I was just creating the PR when I saw you beat me to it by three minutes :P

view this post on Zulip Joshua Nelson (Dec 04 2020 at 01:22):

oops, sorry :laughing:


Last updated: Oct 21 2021 at 19:46 UTC