C++: Reorder some predicates in SSAConstruction

[MathiasVP]

When comparing on https://asgerf.github.io/codeql-flamegraph-viewer/, this reduces the tuples from dominated dependencies number from the IR stage by about 10% when running on OpenJDK. I don't know if it has a measurable impact, but it's worth a try!

CPP-Differences: https://jenkins.internal.semmle.com/job/Changes/job/CPP-Differences/2100/

[geoffw0]

Changes look about as safe as they come. Would like to see a bit more performance information, but the CPP Differences job appears to have failed.

[MathiasVP]

Would like to see a bit more performance information, but the CPP Differences job appears to have failed.

Indeed. I'll run a new job once Jenkins recovers.

[intrigus-lgtm]

OOI: Why/How does this work?
I may be missing some context here, but how can reordering parameters lead to a speed-up?

[MathiasVP]

OOI: Why/How does this work?
I may be missing some context here, but how can reordering parameters lead to a speed-up?

Basically, this piece of hypothetical QL:

foo(a, b) and bar(a, b)

will be compiled into this slightly simplified piece of relational algebra:

result = JOIN foo WITH bar ON FIRST 2 OUTPUT foo.0, foo.1

i.e., give me the tuples (a, b) such that foo(a, b) and bar(a, b) are satisfied. However, when we need to compute this very related piece of QL:

foo(a, b, c) and bar(b, c)

the relational algebra (again, simplified) will be something like (assuming we only need the second and third column):

foo_tmp = SCAN foo OUTPUT foo.1, foo.2
result = JOIN foo_tmp WITH bar ON FIRST 2 OUTPUT foo_tmp.1, foo_tmp.2

Notice that we need a SCAN in order to reorder the parameters of foo before we can compute the join. However, if we instead reorder the parameters of foo so that the QL code looks like this:

foo(b, c, a) and bar(b, c)

then the (simplified) relational algebra will again be a nice and simple join:

result = JOIN foo WITH bar ON FIRST 2 OUTPUT foo.0, foo.1

Does that make sense?

[geoffw0]

Again, the changes look safe but we're still waiting for some actual performance numbers (I'd be happy with a slight improvement, or no change + tuple counts appear to be lower).

[MathiasVP]

Again, the changes look safe but we're still waiting for some actual performance numbers (I'd be happy with a slight improvement, or no change + tuple counts appear to be lower).

A run on 5a62cbe finished and showed no real difference. I noticed that the changes I made generated a couple of new unnecessary SCANs which I've fixed in b138e1e. That one should hopefully fare better.

[geoffw0]

Code changes still LGTM.

[geoffw0]

(sorry I missed the getAFeasibleSuccessor() mistake!)

[MathiasVP]

(sorry I missed the getAFeasibleSuccessor() mistake!)

No worries 😄

[MathiasVP]

It looks like there's no real performance gain here. In fact, there might be a slight performance regression which I can't quite explain. I'll close this PR for now. It might be worth investigating the affected RA more closely in the future.

[geoffw0]

That's a shame, the idea seems quite promising.