Rework NFA matching and try with resources filtering #1020
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 9 commits
May 10, 2022 09:26
The slot storage system in the NFA matching library was a bit of hack, storing any matched values even if that branch do not match. Working around this complicated using the matching library as the behaviour was not intuitive. This change fixes the implementation so updates to the context are tied to the state that made them. The behaviour is now 'correct' and much easier to understand. Unfortunately this has two consequences. 1. It is much slower. A 1 second analysis on an example project now takes 8 seconds. 2. The try with resources filter is broken as it relied on peculiar behaviour of the original implementation. Some mitigation has been put in place for the first point. Previously the filters were run in the prescan (because they were so cheap). Most filters will now not be run, unless they have the new type of PRESCAN_FILTER. Before this can be merged the try with resources filter must be fixed and further benchmarks taken with large projects to understand the impact.
Rework filter based on the analysis done by the jacoco team. Also puts mitigation in place against the increased analysis cost of properly branching the filter context.
A large amount of static analysis time is spent computing hashcodes for maps. A significant performance improvement can be made by creating specialised classes for the common cases of contexts with 0 or 1 values.
hcoles
force-pushed
the
feature/extract_state_in_sequence_match
branch
from
b209fc0 to
dfc8d5d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The slot storage system in the NFA matching library was a bit of hack,
storing any matched values even if that branch do not match. Working
around this complicated using the matching library as the behaviour was
not intuitive.
This change fixes the implementation so updates to the context are tied
to the state that made them. The behaviour is now 'correct' and much
easier to understand.
Unfortunately the analysis is now much slower to execute (8 seconds vs < 1 seconds on one example project) if the queries are left unmodified. To ensure that this will scale as the size of the input project grows, mitigation has been put in place. Previously the filters were run in the prescan (because they were so cheap). Most filters will now not be run, unless they have the new type of PRESCAN_FILTER. Some queries have also been rewritten.
The try with resources filter relied on the previous broken behaviour and has been re-written based on the analysis used in jacoco. This should now detect cases previously missed.