Navigation seems to slow down auto complete

[srawlins]

When I have a certain file open (internally), and run a CPU profile while trying to autocomplete, I get that we're spending > 500 ms in Directory.exists:

It seems like _DartNavigationComputerVisitor calls resourceProvider.getFile for every doc comment visited in visitComment. This was introduced in https://github.com/dart-lang/sdk/commit/ae7e3517c80532eaac28c8d718e5caaf06722b46#diff-31f9a5232da9a94e12badc4bff0d6a6212b70984059e0dcab30c60e94243c510. It seems bad to me.

@scheglov can you say why _DartNavigationComputerVisitor would be doing work at this moment? Here are the steps I take to reproduce (with dummied down text):

1. File contains something like class C { final _x = {}; void f() { _x.entries.^ } } with cursor at the caret.
2. I start a CPU profile, in DevTools
3. I type <backspace>
4. I type .
5. I wait several seconds.
6. completions appear for instance members on List.

[srawlins]

CC @gspencergoog who wrote the referenced change and @bwilkerson who might be familiar as well.

[scheglov]

Yes, doing IO, is a very bad idea. Even if there is a need to get some additional data, this data should be cached and not recomputed every time.

I suspect that _DartNavigationComputerVisitor does this work because we wanted to implement some feature. I'm not sure if you had a more specific question that I fail to recognize.

[bwilkerson]

That could easily be optimized. We don't need nearly as many existence checks, and we could probably cache the result temporarily while processing a single compilation unit (or maybe even longer term).

I have no idea why _DartNavigationComputerVisitor is running during code completion (or whether it is). Can you expand the call stack and find out where it's being run from?

[bwilkerson]

_DartNavigationComputerVisitor is doing the work so that we can navigate from the link to an example to the source for the example in Flutter-style doc comments.

[srawlins]

@scheglov

I'm not sure if you had a more specific question that I fail to recognize.

I was wondering why _DartNavigationComputerVisitor is entered during the steps I mentioned.

[scheglov]

The reason _DartNavigationComputerVisitor is running during completion might be that completion is an async operation, so it can be interleaved with other operations running in parallel.

[srawlins]

I'll ask a different question: What IDE events trigger entering _DartNavigationComputerVisitor? Clicking the mouse around? Typing a character into a file?

[bwilkerson]

Even if it isn't interleaved sometimes, this could sometimes account for the latency we're seeing. If server has started computing navigation before the request for code completion is seen, and it's taking a long time for navigation to return, then it's might be preventing code completion from getting started.

What IDE events trigger entering _DartNavigationComputerVisitor?

Modifying a file.

[srawlins]

Here is the rest of (a different repro of) the call to DartNavigationComputerVisitor:

[scheglov]

Typing a character. This changes the file overlay, which triggers file analysis, and when the resolved unit is received by the server, it starts computing notifications, such as the navigation and semantic highlighting.

[bwilkerson]

Sounds like we need a way to delay computing the notifications when a code completion request is already on the queue so that these tasks won't interfere with completions. Kind of sounds like a job for a scheduler.

[srawlins]

Ah that's a good point. Since I'm typing two things: <backspace> and then ., the requests might go something like:

1. user types <backspace>
2. IDE requests completions at _x.entries^
3. IDE requests navigations
4. etc.
5. user types .
6. IDE requests completions at _x.entries.^

so one or more navigation requests are in the pipe. And may be in progress, so they can't be cancelled.

[srawlins]

OK we will revert https://github.com/dart-lang/sdk/commit/ae7e3517c80532eaac28c8d718e5caaf06722b46 unless someone jumps on a fix forward, to dramatically reduce the introduced file system I/O.

[srawlins]

Looking at the code more, we call out to the file system for every import directive and export directive, in _addUriDirectiveRegion, during navigation-computation (on every edit to a file).

[bwilkerson]

Which probably wouldn't be a problem if we were cancelling obsolete navigation requests and scheduling them to happen after code completion requests.

[gspencergoog]

You're right that it shouldn't be doing it for every doc comment. My intention was to do it once per compilation unit, and cache it.

The code does attempt to cache some results per compilation unit. It sets _examplesApiPath on the visitor, which is the path to the examples directory, and bypasses existence checks if it has already found it. Maybe I misunderstood the lifetime of the visitor though: can I use a value on the visitor as a cache if I clear it out every time I get a new call to visitCompilationUnit? If not, where would be a better place to cache it?

However, it should probably also bypass the existence check if the doc comment doesn't include an example, which trades running a string search in every doc comment for one I/O access per compilation unit, which is probably a good trade.

[bwilkerson]

The lifetime of _DartNavigationComputerVisitor is the duration of computeDartNavigation, so you don't need to clear the cache.

Yes, a string search is much faster than file I/O and will probably take care of the performance issue.

[srawlins]

I have https://dart-review.googlesource.com/c/sdk/+/271863 mailed