Skip to content

PS: Fix two bad joins#355

Open
MathiasVP wants to merge 1 commit intomainfrom
fix-joins-in-powershell
Open

PS: Fix two bad joins#355
MathiasVP wants to merge 1 commit intomainfrom
fix-joins-in-powershell

Conversation

@MathiasVP
Copy link
Copy Markdown
Collaborator

This PR fixes two unrelated bad joins.

The first issue is in control-flow:

Tuple counts for _BasicBlocks::BasicBlock.getANode/0#dispred#ef46d058_CfgNodes::ChildMapping.reachesBasicBlock/3#disp__#antijoin_rhs/4@i5#L0#2661dw6a after 3m51s:
  46822430   ~0%     {4} r1 = SCAN `CfgNodes::ChildMapping.reachesBasicBlock/3#dispred#60bda98b#prev_delta` OUTPUT In.1 'arg1', In.0 'arg0', In.2 'arg2', In.3 'arg3'
  6129103632 ~4%     {5}    | JOIN WITH ControlFlowGraphImpl::Cached::TAstNode#2a844ba8_13#join_rhs ON FIRST 1 OUTPUT Lhs.3 'arg3', Rhs.1, Lhs.1 'arg0', Lhs.0 'arg1', Lhs.2 'arg2'
  586522     ~1%     {4}    | JOIN WITH `BasicBlocks::BasicBlock.getANode/0#dispred#ef46d058` ON FIRST 2 OUTPUT Lhs.2 'arg0', Lhs.3 'arg1', Lhs.4 'arg2', Lhs.0 'arg3'
                     return r1

Instead of joining early on the second (first) column of TAstNode we now filter out the irrelevant columns to reduce the duplication:

Tuple counts for CfgNodes::getARelevantBasicBlock/1#44f5f003/2@aa3dc3oj after 1.6s:
  15314483 ~0%     {2} r1 = SCAN cached_ControlFlowGraphImpl::Cached::TAstNode#2a844ba8 OUTPUT In.3, In.1 'a'
  15314483 ~0%     {2}    | JOIN WITH `BasicBlocks::BasicBlock.getANode/0#dispred#ef46d058_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1 'a', Rhs.1 'result'
                   return r1

Tuple counts for CfgNodes::ChildMapping.reachesBasicBlock/3#bccb8ab5/4@i3#5b199wnb after 7m26s:
  17648632  ~1%     {4} r1 = SCAN `CfgNodes::ChildMapping.reachesBasicBlock/3#dispred#60bda98b#prev_delta` OUTPUT In.1 'child', In.3, In.0 'this', In.2 'cfn'
                    {4}    | AND NOT `CfgNodes::getARelevantBasicBlock/1#44f5f003`(FIRST 2)
  5601517   ~0%     {4}    | SCAN OUTPUT In.2 'this', In.0 'child', In.3 'cfn', In.1
  ...

The second problem fixed is that we were misusing ContentSet in readStep:

predicate readStep(Node node1, ContentSet c, Node node2) {
 ...
  exists(
    CfgNodes::StmtNodes::ForEachStmtCfgNode forEach, Content::KnownElementContent ec, BasicBlock bb,
    int i
  |
    c.isKnownOrUnknownElement(ec) and
    ...
  )
}

This caused a large blowup on a particular database. The problem is that this that, while this is correct, it's not how the ContentSet is supposed to be defined by this predicate (for performance reasons). Instead, we can use use c.isAnyPositional() to select the "set of contents" that correspond to positional Contents (i.e., what you can iterate over using foreach statements).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant