Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: short circuit logic nodes when appropriate #827

Merged
merged 44 commits into from
Nov 9, 2021

Conversation

williballenthin
Copy link
Collaborator

@williballenthin williballenthin commented Nov 8, 2021

closes #824
review and merge #828 first

Adds logic to and, or, and some statements and substring and regex features to detect when the nodes are minimally satisfied (or not) and to complete evaluation early. For example, if one child of an and statement fails, then the and statement will never be satisfied, and the remaining children don't actually have to be evaluated.

However, in some cases "thorough" evaluation may still be desirable, such as with or statements that have multiple children that may be satisfied. In a verbose output mode, users may want to see all the evidence related to a rule match, not just the minimal set of evidence.

Therefore, this PR includes logic to invoke the (fast) short circuiting mode first, and only if there's a match (uncommon), go back and collect the thorough results for display.

label count(evaluations) avg(time) min(time) max(time)
6c8d246 base 108,121 0.37s 0.31s 0.41s
ad119d7 short circuiting (always) 69,211 0.20s 0.16s 0.28s
3e74da9 short circuiting (hybrid) 69,401 0.25s 0.24s 0.29s

(via: PMA01-01, 30 iterations)

In the above table, we see that the PR here matches about 30% faster in this test case. When always short circuiting (at the expense of non-thorough results) then we can go a bit faster.

As expected, there are more node evaluations in the "hybrid" mode, as the engine goes back and collects thorough results once a match has been found.

Checklist

  • No documentation update needed

@williballenthin williballenthin added the enhancement New feature or request label Nov 8, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed

@github-actions github-actions bot dismissed their stale review November 8, 2021 21:13

CHANGELOG updated or no update needed, thanks! 😄

@williballenthin williballenthin marked this pull request as ready for review November 8, 2021 22:17
capa/engine.py Show resolved Hide resolved
capa/engine.py Outdated Show resolved Hide resolved
capa/engine.py Outdated Show resolved Hide resolved
capa/engine.py Outdated Show resolved Hide resolved
williballenthin and others added 3 commits November 9, 2021 10:48
Co-authored-by: Moritz <mr-tz@users.noreply.github.com>
Co-authored-by: Moritz <mr-tz@users.noreply.github.com>
Co-authored-by: Moritz <mr-tz@users.noreply.github.com>
@williballenthin williballenthin merged commit 9350ee9 into master Nov 9, 2021
@williballenthin williballenthin deleted the perf/short-circuit branch November 9, 2021 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

short circuit evaluation of logic statements
2 participants