Dataflow: Refactor FlowState to be paired with Node

[aschackmull]

This refactors the representation of FlowState in the data flow library. Stage 1 is extracted to its own file and then for subsequent stages we either use NodeEx directly or a (NodeEx, FlowState) pair as the node type in order to eliminate the FlowState column.

[aschackmull]

Commit-by-commit review encouraged, but the second commit really needs to be viewed with git diff --minimal as that saves about 6000 lines of superfluous diff output.

[hvitved]

Nice refactor, trivial comments only. Also, performance needs to be fixed.

[hvitved]

Should this be moved up before the Stage1 module, perhaps?

[aschackmull]

It contains a bit of a mix of input and output predicates relative to the Stage1 module. It's mostly output, though, hence why I put it after.

[hvitved]

This comment (and the end comment) is perhaps redundant now?

[aschackmull]

Yes, I'll remove them.

[hvitved]

QL doc should probably also mention the Stage1 parameter.

[asgerf]

Wouldn't it be cheaper and simpler to pair it up with the access path instead? I imagine the TNil case storing the FlowState (with similar adaptions to the various AP approximations):

newtype TAccessPath =
  TNil(FlowState state) { ...} or
  TCons(Content head, TAccessPath tail) { ... }

[aschackmull]

Wouldn't it be cheaper and simpler to pair it up with the access path instead?

I don't think so. It might be cheaper in terms of number of constructed elements of IPA types, but I definitely don't think it's simpler. Firstly, doing so would be a semantic change in all the cases where we aren't tracking an exact access path. Secondly, the tracking of FlowStates would then be different in each stage and that would mean that stages 2-6 would still need to care about and handle FlowState, whereas the point of the present refactor is that we can handle FlowState once and for all between stages 1 and 2, and then the bulk of the data flow library simply doesn't need to ever care about FlowState, which I think is a major improvement in terms of reducing the complexity of the library. In particular, it has become much easier to see that the stateful in- and out-barriers are now handled correctly. That's a huge pain in the current implementation.

[aschackmull]

Dca looks good now.

[hvitved]

There is an impressive JS speedup on microsoft__vscode; is that because of db1ed67? (cc @asgerf ).

[aschackmull]

There is an impressive JS speedup on microsoft__vscode; is that because of db1ed67? (cc @asgerf ).

I think it might simply be due to reduced memory pressure, since we've removed a column from most predicates. Impressive nonetheless.
Additionally a few predicates now track state, which they didn't before, and that yields a very minor precision improvement that reduces the tuple count a little. Testing locally, I couldn't get the same speedup, but that may be because I simply have more ram available.

[asgerf]

There is an impressive JS speedup on microsoft__vscode; is that because of db1ed67?

That's great news! The stage timings seem to indicate js/insecure-randomness and to a lesser degree js/xss as the main contributors. Insecure randomness has long been a problematic query for that project, so it would be interesting to find out why it became faster.

[aschackmull]

I just checked js/insecure-randomness and looked at it with the "Compare Performance" feature in VSCode. The largest predicate, Stage4::fwdFlow1 computes fewer tuples for some reason, but otherwise it looks fairly similar, so I think reduced ram-pressure is the main factor in the speedup. That query computes a huge number of tuples in stage 4, even more than js/xss.