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

Somewhat cache DFA results #2924

Merged
merged 1 commit into from Oct 10, 2019
Merged

Somewhat cache DFA results #2924

merged 1 commit into from Oct 10, 2019

Conversation

dotpaul
Copy link
Contributor

@dotpaul dotpaul commented Oct 10, 2019

Partially fixes #2900

From my observations with slow analysis with DoNotHardCodeEncryptionKey, the root cause is a combination of ValueContent analysis can be slow and ValueContentAnalysisResult cache misses, due to the limited DFA result cache. That causes ValueContent analysis to be invoked multiple times.

This somewhat mitigates the problem, by caching analysis on a per-analyzer-operationblock basis, in TaintedData analyzers that use ValueContentAnalysis.

Longer term we should see if we can come up with a way of caching DFA results on a per compilation basis without memory leaks, and see what we can do to optimize ValueContent analysis.

@dotpaul dotpaul merged commit fb3ada2 into dotnet:2.9.x Oct 10, 2019
@mavasani
Copy link
Member

Longer term we should see if we can come up with a way of caching DFA results on a per compilation basis without memory leaks, and see what we can do to optimize ValueContent analysis.

I am still concerned about significant memory usage that comes along with holding all DFA results for a compilation in memory. IMO, we need some hybrid solution here.

@dotpaul dotpaul deleted the somewhatCacheDfa branch October 10, 2019 21:18
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.

None yet

3 participants