dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.28k stars 1.58k forks source link

Feature idea: detect when many file changes occur at once, and wipe state clean #48746

Open srawlins opened 2 years ago

srawlins commented 2 years ago

This is a vague idea right now because I don't super know what state analysis server holds onto which is individually invalidated on a file change.

But we've received a good amount of feedback that analysis server crashes, eats CPU, eats memory, when many files change "at once". The two examples are (A) switching git branches which have a lot of changes between them, and (B) running build_runner which re-generates many files.

I haven't reproduced either of these states yet, so that's a good first step. But assuming it reproduces, my idea is that there is something expensive about re-analyzing many files, when there is not something expensive about analyzing the same files the first time. Perhaps resource leaks (file handles?), or multiple changes per file triggering the event loop more than once per file path.

Whatever the case, if we can detect that many files have changed "at once," can we switch to a new behavior, of wiping our slate clean and re-analyzing everything from scratch?

It might take some design work and experimentation to figure out the heuristic (% of files currently under analysis? minimum number?), but the feedback is clear that a good percentage of Flutter developers are very impacted by this issue.

bwilkerson commented 2 years ago

@scheglov

scheglov commented 2 years ago

I also see that when there are many changes at once, we get into the state when the state of the world as we think it is (in form of the element model) is not what it actually is. So, it is kind of a race condition, or missing a dependency. We probably start analyzing one file, and while doing this, a few more files change, and we read them without realizing that we need to invalidate some libraries. This causes crashes. It might get fixed when we switch AnalysisDriver._applyFileChangesSynchronously in the breaking change.

I don't remember reports of extra CPU or heap usage because of many files changed. It might be possible that invalidating analysis results for a changed file is expensive, so when there are many files changed, we spend too much on trying to being more precise, and it would be faster to restart. But I have not seen such situations while working on analyzer and analysis server. It is always analysis that is slow. And my expectation is that restarting will only make things slower - we will have to read again files, decode summaries into element models.

Heap usage might go down after restart. The reason is that as we analyze, we might require some libraries, so we resynthesize them from summaries, but if we don't need them anymore, we will keep them in memory anyway. This is probably not an issue for DAS, because we use element models of almost all libraries for many semantic features - completion, quick fixes, etc.

srawlins commented 2 years ago

OK let's defer until the breaking change. In the meantime, I would like to have a consistent reproduction of the issues that occur when many files change (user reports include stale Problems panel, hanging, high memory).