Python: Fix bad join in method call order computation#21429
Conversation
This join had badness 1127 on the project FiacreT/M-moire, producing ~31 million tuples in order to end up with only ~27k tuples later in the pipeline. With the fix, we reduce this by roughly the full 31 million (the new materialised helper predicate accounting for roughly 130k tuples on its own). Co-authored-by: Mathias Vorreiter Pedersen <mathiasvp@github.com>
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
Seems to preserve the functionality.
There was a problem hiding this comment.
Pull request overview
Improves performance of the Python QL “method call order” computation by factoring out a costly superclass-check into a dedicated helper predicate, reducing an expensive join that was producing a large number of intermediate tuples.
Changes:
- Replaces an inline
exists(Class superBase | ...)check with a new helper predicate to avoid an inefficient join pattern. - Adds a
pragma[nomagic]private predicate to encapsulate the “superclass also missing call” condition.
You can also share your feedback on Copilot code review. Take the survey.
| * Holds if a strict superclass of `base` is also missing a call to `shouldCall` named `name`, | ||
| * indicating that `base` is not the highest class in the hierarchy with this issue. | ||
| */ |
There was a problem hiding this comment.
The new doc comment is a bit ambiguous: “missing a call to shouldCall named name” reads like shouldCall is being named here. Consider rephrasing to explicitly say this checks whether any strict superclass of base also satisfies missingCallToSuperclassMethod(..., shouldCall, name) (i.e., is also missing the call to the superclass method name).
| * Holds if a strict superclass of `base` is also missing a call to `shouldCall` named `name`, | |
| * indicating that `base` is not the highest class in the hierarchy with this issue. | |
| */ | |
| * Holds if some strict superclass of `base` also satisfies `missingCallToSuperclassMethod(..., shouldCall, name)`, | |
| * that is, is also missing the call to the superclass method named `name`, so `base` is not the highest class | |
| * in the hierarchy with this issue. |
This join had badness 1127 on the project FiacreT/M-moire, producing ~31 million tuples in order to end up with only ~27k tuples later in the pipeline. With the fix, we reduce this by roughly the full 31 million (the new materialised helper predicate accounting for roughly 130k tuples on its own).