-
Notifications
You must be signed in to change notification settings - Fork 464
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
Enable concurrent execution #6357
base: main
Are you sure you want to change the base?
Conversation
I don't think we want to try updating the 2.9.x branch. It has been dormant for quite a while and unless there is an absolute need we want to avoid fixing anything here. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6357 +/- ##
=======================================
Coverage 96.11% 96.11%
=======================================
Files 1361 1361
Lines 316034 316035 +1
Branches 10192 10192
=======================================
+ Hits 303741 303746 +5
+ Misses 9861 9858 -3
+ Partials 2432 2431 -1 |
{ | ||
//analysisContext.EnableConcurrentExecution(); | ||
context.EnableConcurrentExecution(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it is safe to enable it now? (The reference in the issue had a comment // TODO: Make analyzer thread-safe.
Could not find from history how its removed or who removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #2068
@mavasani I'm assuming 2.9.x is the correct branch? Let me know if I should rebase on main branch
Note: I don't see any concurrency issues in the implementation, but this needs to be carefully confirmed.