Skip to content

Conversation

@jbj
Copy link
Contributor

@jbj jbj commented Nov 29, 2018

This predicate was fast with the queries and engine from 1.18. With the queries from master it got a bad join order in the UninitializedLocal.ql query, which made it take 2m34s on Wireshark. This commit decomposes bbEntryReachesLocally into two predicates that together take only 4s.

This predicate was fast with the queries and engine from 1.18. With the
queries from `master` it got a bad join order in the
`UninitializedLocal.ql` query, which made it take 2m34s on Wireshark.
This commit decomposes `bbEntryReachesLocally` into two predicates that
together take only 4s.
@jbj jbj added the C++ label Nov 29, 2018
@jbj jbj added this to the 1.19 milestone Nov 29, 2018
@jbj jbj requested a review from a team as a code owner November 29, 2018 14:21
@geoffw0
Copy link
Contributor

geoffw0 commented Nov 29, 2018

LGTM, though at this stage I'd definitely like to give it a try before merging...

Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two wireshark snapshots on my machine. On one of them this speeds it up from 807s to 633s, by making bbEntryReachesLocally disappear from the list of most expensive predicates. The other snapshot apparently hangs around 520s with much less contribution from bbEntryReachesLocally (perhaps it's missing something pathological like a giant test case). In any case this seems to be doing what it promises.

@geoffw0 geoffw0 merged commit 4744cec into github:rc/1.19 Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants