Python: Fix another bad "value transfer" join #9859
Conversation
The culprit:
```
Tuple counts for PointsTo::InterProceduralPointsTo::scope_entry_value_transfer_from_earlier#741b54e2#ffff#join_rhs/5@eb1340iv after 12.6s:
72973 ~3% {2} r1 = JOIN PointsToContext::TImportContext#cf3039a0#f WITH Definitions::NonEscapingGlobalVariable#class#486534ab#f CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0 'arg1'
537932 ~0% {3} r2 = JOIN r1 WITH Essa::EssaDefinition::getSourceVariable#dispred#f0820431#ff_10#join_rhs ON FIRST 1 OUTPUT Rhs.1 'arg2', Lhs.1 'arg1', Lhs.0
982333 ~0% {4} r3 = JOIN r2 WITH Essa::EssaVariable::getAUse#dispred#f0820431#ff ON FIRST 1 OUTPUT Lhs.2, Lhs.1 'arg1', Lhs.0 'arg2', Rhs.1 'arg0'
37029774 ~0% {4} r4 = JOIN r3 WITH Essa::TEssaNodeDefinition#24e22a14#ffff ON FIRST 1 OUTPUT Rhs.3 'arg3', Lhs.1 'arg1', Lhs.2 'arg2', Lhs.3 'arg0'
35956211 ~0% {5} r5 = JOIN r4 WITH Essa::ScopeEntryDefinition::getScope#dispred#f0820431#ff ON FIRST 1 OUTPUT Lhs.3 'arg0', Lhs.1 'arg1', Lhs.2 'arg2', Lhs.0 'arg3', Rhs.1 'arg4'
return r5
```
You may notice that this is a predicate that's _materialised_, but it's
never actually used anywhere. It's the old "standard order" bringing
much sadness.
The problem here is that in the standard order (which we never actually
use here), we end up with a join between the bits above, `getRootCall`,
and `appliesToScope`. The `join_rhs` bit is joined twice, once with
`getRootCall#prev` and `appliesToScope#prev_delta` (in that order), and
once with `prev` and `prev_delta` swapped.
So to fix this, I used the unbinding pragma to force `appliesToScope` to
appear first in the join order. This was enough to make the compiler
_not_ push the common context into its own `join_rhs` predicate (and
the join-order is still decent.)
There was a problem hiding this comment.
I had to get some help from @aschackmull to understand what is going on here. It seems that you are basically forcing the delta to be early by unbinding the recursive calls. This assuming that appliesToScope and getScope are the recursive calls (I could not really verify this, even browsing through the source code). If this is the explanation, then creating non_escaping_global_transfer as its own predicate should be unnecessary.
If you are up for it, please try with just the added pragmas (unless you already did). If you do not feel like spending that kind of time on points-to code, let me know and I will approve as is.
|
As we discussed offline (but for the edification of anyone following this PR), I will not be testing the suggested change, as the current fix is sufficient, and there are more pressing matters to attend to. |
The culprit:
You may notice that this is a predicate that's materialised, but it's never actually used anywhere. It's the old "standard order" bringing much sadness.
The problem here is that in the standard order (which we never actually use here), we end up with a join between the bits above,
getRootCall, andappliesToScope. Thejoin_rhsbit is joined twice, once withgetRootCall#prevandappliesToScope#prev_delta(in that order), and once withprevandprev_deltaswapped.So to fix this, I used the unbinding pragma to force
appliesToScopeto appear first in the join order. This was enough to make the compiler not push the common context into its ownjoin_rhspredicate (and the join-order is still decent.)