-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ConstraintSystem] Change the order in which we attempt disjunctions to be stable. #17975
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
Conversation
disjunction is used.
to be stable. We currently will stop visiting the elements of a disjunction under certain circumstances once we have found a solution. The result we get is inherently dependent on the order in which we determine to visit the disjunctions themselves (in addition to the elements of the disjunction). This change makes the order in which we visit disjunctions stable. Future commits will create a stable ordering for the elements of disjunctions. Once we also have that stable ordering in place we can in theory short circuit more often as part of changing the way in which we decide what the "best" solution is to a system. This results in an expression in validation-test/stdlib/AnyHashable.swift.gyb no longer being able to typecheck in a reasonable amount of time, so I had to tweak that expression.
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
lib/Sema/CSSolver.cpp
Outdated
// overload sets. | ||
// Pick the disjunction with the lowest disjunction number in order | ||
// to solve them in the order they were created (which should be | ||
// stable within an expression). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean abandoning the idea of finding a better heuristic?
I've been wondering if it'd be possible to use a smarter heuristic here. One that takes into account not just disjunction size, but also
- the dependencies between the disjunctions (to minimize backtracking)
- the relative restrictiveness of their related constraints.
Would the approach you're taking here preclude that?
(Or maybe that approach is an "obviously" bad idea. If so, I'd be grateful for any recommendations for things to read up on.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a fixed ordering by definition precludes trying other orderings.
We've considered and tried some other heuristics here and they have shown some promise, but have not really provided significant benefit.
The path I am looking at now is:
- Fix the ordering that we traverse in based on the user-written expression (or at least the way we walk them in the compiler).
- Rather than removing short-circuiting that we do, double-down on the notion that we should stop solving once we have a clear "best" solution.
- Determine the "best" solution primarily by comparing overloads we're attempting in a given disjunction. My plan is to make a partial ordering of overloads using
isDeclAsSpecializedAs
, and then having a sort of hierarchy of disjunctions based on that ordering (i.e. three incomparable overloads would all be at the same level in the hierarchy, ones that are clearly less specialized would be at the next level - all the overloads would form a poset). - All the elements of the disjunction at the same level would be attempted, and if any results in a successful solution, stop. If more than one does, then use secondary criteria to determine a winner. If none does, move on to the next level in the hierarchy.
- Where the current scoring mechanism fits into this is TBD, but my hope is that we can eventually remove the short-circuiting based on score, and instead use that as one of the tie-break criteria. Some initial experimentation also leads me to believe that we'll need to use the score in some way to decide whether the solutions we find in one level of the hierarchy are clear winners.
The hope is that this newer sort of short circuiting will be more predictable and easier to reason about as well as allowing us to bail out earlier and come up with a final solution faster as a result.
@swift-ci Please smoke test |
@swift-ci Please test source compatibility |
We currently will stop visiting the elements of a disjunction under
certain circumstances once we have found a solution. The result we get
is inherently dependent on the order in which we determine to visit
the disjunctions themselves (in addition to the elements of the
disjunction).
This change makes the order in which we visit disjunctions
stable. Future commits will create a stable ordering for the elements
of disjunctions. Once we also have that stable ordering in place we
can in theory short circuit more often as part of changing the way in
which we decide what the "best" solution is to a system.
This results in an expression in
validation-test/stdlib/AnyHashable.swift.gyb no longer being able to
typecheck in a reasonable amount of time, so I had to tweak that
expression.