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.

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

librustc: Introduce a "quick reject" mechanism to quickly throw out #17995

Closed
wants to merge 1 commit into from

Conversation

pcwalton
Copy link
Contributor

method candidates and prevent method lookup from being invoked at all
for overloaded operators.

40%-50% improvement in typechecking time.

r? @nikomatsakis

method candidates and prevent method lookup from being invoked at all
for overloaded operators.

40%-50% improvement in typechecking time.
@@ -586,6 +586,11 @@ pub struct ctxt<'tcx> {

/// Caches the representation hints for struct definitions.
pub repr_hint_cache: RefCell<DefIdMap<Rc<Vec<attr::ReprAttr>>>>,

pub overloaded_operator_filter:
RefCell<HashMap<SimplifiedType,OverloadedOperatorFilter>>,
Copy link
Member

Choose a reason for hiding this comment

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

This should use FnvHashMap. I wonder if there are other places in the compiler where randomized HashMap's have been used, and which are causing non-determinism.

@aturon
Copy link
Member

aturon commented Oct 13, 2014

This is awesome! 💨

@nikomatsakis
Copy link
Contributor

Reading the patch I'm finding it hard to reverse engineer the high-level idea, and in particular the interplay between the smart pointer table and the allow/disallow deref flags. Would it be possible to create a few representative examples and add them as comments onto the main lookup fns to convey what is going on?

This looks cool though -- it may be subsumed by the more general caching mechanism I had in mind, but then again maybe not.

@pcwalton
Copy link
Contributor Author

@nikomatsakis The idea here is to be able to quickly rule out methods that cannot match no matter what. The idea is loosely inspired by CSS selector matching: instead of having a list of methods and having to go to the trouble of creating type variables and unifying self types, we introduce a "base type" (here called SimplifiedType) that all methods are "keyed off". The idea here is that if basetype(callee type) != basetype(self type of impl), then callee type cannot possibly match self type of impl. To account for autoderef/autoref, base types "look through" pointers, so &int, &&int, and Box<int> will all have the base type int. Not all types have base types (for example, tuples); if no base type can be determined the full, most general unification code is used. However, that is rare (less than 10% of cases).

There is a complication here in that for smart pointers methods can be called on either the smart pointer itself or the referent type. That is why smart pointers actually have two "base types"—one for the smart pointer and one for the type it points to—and methods are considered to possibly match if they have either the base type of the smart pointer or the base type of the referent.

We also use this same idea to rule out trying overloaded operators if they cannot possibly match. Autoderef doesn't happen for overloaded operators though, so that's why we have allow/disallow deref flags.

Does this make more sense?

@nikomatsakis
Copy link
Contributor

@pcwalton Yes. I got most of it but the smart pointer stuff I didn't quite see in the code -- though what you described is roughly what I had guessed. I'll take another look and see if I can figure it out. I was planning to do a more general cache; I wonder if this approach replaces and/or complements that idea. (I mean, clearly you can do both, but it's not clear that a cache adds much over this in terms of execution time; it's also unclear if a cache alone would do better than this or worse). Guess the only way to be sure would be to try it out.

@nikomatsakis
Copy link
Contributor

I'm going to try and rebase it against master, which ought to be a good way to also make sure I understand it.

bors added a commit that referenced this pull request Nov 8, 2014
…2, r=nrc

This is a pretty major refactoring of the method dispatch infrastructure. It is intended to avoid gross inefficiencies and enable caching and other optimizations (e.g. #17995), though it itself doesn't seem to execute particularly faster yet. It also solves some cases where we were failing to resolve methods that we theoretically should have succeeded with.

Fixes #18674.

cc #18208
@nikomatsakis
Copy link
Contributor

I plan to roll this into #18694 in an effort to improve memory usage on that branch. I had planned to do this separately, but I think it will help unblock that PR by reducing memory usage, since quick-rejecting impls means we should generate fewer types too I think.

@alexcrichton
Copy link
Member

Closing in favor of #18694.

bors added a commit that referenced this pull request Nov 17, 2014
…2, r=nrc

This is a pretty major refactoring of the method dispatch infrastructure. It is intended to avoid gross inefficiencies and enable caching and other optimizations (e.g. #17995), though it itself doesn't seem to execute particularly faster yet. It also solves some cases where we were failing to resolve methods that we theoretically should have succeeded with.

Fixes #18674.

cc #18208
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants