Skip to content

[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

Merged
merged 3 commits into from
Jul 16, 2018
Merged

[ConstraintSystem] Change the order in which we attempt disjunctions to be stable. #17975

merged 3 commits into from
Jul 16, 2018

Conversation

rudkx
Copy link
Contributor

@rudkx rudkx commented Jul 16, 2018

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.

rudkx added 2 commits July 16, 2018 08:49
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.
@rudkx
Copy link
Contributor Author

rudkx commented Jul 16, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jul 16, 2018

@swift-ci Please test source compatibility

// 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).
Copy link
Contributor

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

  1. the dependencies between the disjunctions (to minimize backtracking)
  2. 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.)

Copy link
Contributor Author

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.

@rudkx
Copy link
Contributor Author

rudkx commented Jul 16, 2018

@swift-ci Please smoke test

@rudkx
Copy link
Contributor Author

rudkx commented Jul 16, 2018

@swift-ci Please test source compatibility

@rudkx rudkx merged commit 9f75e4a into swiftlang:master Jul 16, 2018
@rudkx rudkx deleted the disjunction-ordering branch July 16, 2018 22:19
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.

2 participants