Dataflow: Replace stage 3 type pruning with flow-insensitive type pruning. #16785
Conversation
aschackmull
force-pushed
the
dataflow/stage3-notypes
branch
from
d8f8bed
to
8a1cf79
Compare
aschackmull
force-pushed
the
dataflow/stage3-notypes
branch
from
a2ed536
to
ac716e7
Compare
|
I've measured the impact on data flow tuple counts in Java with MRVA on top 1000 using a combined set of 10 representative queries: I defined the cost to be the sum of forward and reverse tuple counts in stages 2 through 6. The general trend shows an improvement - in particular on larger cases.
|
aschackmull
added
the
no-change-note-required
| @@ -564,7 +564,6 @@ predicate neverSkipInPathGraph(Node n) { | |||
| * Holds if `t1` and `t2` are compatible, that is, whether data can flow from | |||
| * a node of type `t1` to a node of type `t2`. | |||
| */ | |||
| pragma[inline] | |||
| predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { any() } | |||
There was a problem hiding this comment.
Why has the stub implementation been changed to bind the parameters for some languages (cpp, go) but not others (python, swift)?
There was a problem hiding this comment.
Because in python and swift DataFlowType is a singleton.
There was a problem hiding this comment.
I know that Go doesn't use types in dataflow, but I forgot that we have a hack in place to avoid an optimiser bug which means DataFlowType isn't a singleton. Apparently I can remove it now.
|
Spot-checked one of the removed results for |
|
Dca looks good. |
aschackmull
force-pushed
the
dataflow/stage3-notypes
branch
from
ac716e7
to
fdf6e30
Compare
|
The CI failure is unrelated and already broken on main. Merging. |
It appears that most of the benefit from type pruning in stage 3 is closely correlated to the
Contentthat's tracked in the access path. This means that we can drop flow-sensitive type pruning, which removes the risk for type-based fanout and consequent performance problems, by replacing it with a flow-insensitive type pruning that merely compares the trackedContentand the type of the current node with the possible container types given by the set of store steps.So far this PR only changes stage 3. How this can impact subsequent changes still needs to be investigated. But this change alone appears to benefit performance - in particular on large databases.