JS: improve the join order of getAClassReference #8550
Conversation
| } | ||
|
|
||
| pragma[noopt] | ||
| private DataFlow::SourceNode getAClassReferenceStep(DataFlow::TypeTracker t) { |
There was a problem hiding this comment.
| private DataFlow::SourceNode getAClassReferenceStep(DataFlow::TypeTracker t) { | |
| private DataFlow::SourceNode getAClassReferenceRec(DataFlow::TypeTracker t) { |
We have other cases of a type-tracking "step" predicates being factored out to fix a join ordering, but where some information about the edge itself is matched. This here is just the whole recursive case being factored, so I'd prefer a different name. I was certainly confused for a second, before realising that the noopt is reason for factoring it out.
|
I made a mistake in the inlining, which caused a test to fail, so this PR might have been for nothing. |
| @@ -970,7 +970,7 @@ class ClassNode extends DataFlow::SourceNode instanceof ClassNode::Range { | |||
| exists(DataFlow::TypeTracker t2, StepSummary summary, DataFlow::SourceNode prev | | |||
| prev = this.getAClassReference(t2) and | |||
| StepSummary::step(prev, result, summary) and | |||
| t2 = t.append(summary) | |||
| t = t2.append(summary) | |||
There was a problem hiding this comment.
Smells like a ql4ql query...
|
I ran an evaluation. No improvement on VSCode (which was the original intent), but other projects see a real performance improvement. I think we should merge this, and I should look into improving the join-order of our type-trackers. |
Saves ~10s on VSCode.
I didn't think such a
nooptshould do a difference, but it did.I'm doing an evaluation, I will only merge if that turns out OK.