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.

Sign up for GitHub

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

Jump to bottom

Completion list takes far too long even after an initial list is generated #4209

Closed
jaredpar opened this issue Jan 17, 2018 · 49 comments
Closed

Completion list takes far too long even after an initial list is generated #4209

jaredpar opened this issue Jan 17, 2018 · 49 comments
Labels
Area-LangService-API Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code.
Milestone
15.7

Comments

@jaredpar
Copy link
Member

jaredpar commented Jan 17, 2018 •
edited by cartermp
Loading

edits by @cartermp

Current issue:

Thanks to @buybackoff for the repro:

  1. Download Spreads.
  2. Build the solution (build.cmd)
  3. Open the solution and theSortedMap.fs file.
  4. Add a new line after line 125, and type this.

Expected: completion list appears reasonably fast, at least certainly after the first try.

Actual: completion appears many seconds after the . every time.

Old issue:

Recently started playing around with VsVim on 15.6 preview 2 and noticed that Intellisense is extremely slow. At first I thought it was broken but eventually found it's just taking ~6-8 seconds to display. This reproduces consistently.

  1. Clone https://github.com/jaredpar/VsVim
  2. Switch to branch repro/fsharp-intellisense-perf
  3. Open VsVim.sln
  4. Open Modes_Visual_VisualMode.fs
  5. Wait for semantic colorizing to take place (ensure initial binding pass is done).
  6. Navigate to line 223. Will have text _selectionTracker.UpdateSelection()
  7. Open a line above and type _selectionTracker.

I would expect Intellisense to popup rather quickly here. Instead it takes ~8 seconds to popup. You can delete the . and retype and see the effect over and over again.

Of possible interest here is this F# project is on the new SDK.
@dsyme
Copy link
Contributor

dsyme commented Jan 18, 2018

That certainly sounds like either

  1. the FCS caching is not taking effect for new-style projects, or
  2. there are a lot of options being recomputed for new-style projects

@jaredpar jaredpar changed the title Intellisense taking ~8 seconds on new SDK in 15.6 Preview 2 Intellisense taking ~8 seconds on new SDK Jan 18, 2018
@jaredpar
Copy link
Member Author

Checked and I'm seeing the same behavior on 15.5.4 as well. Changed title to reflect that it's not specific to the preview.

@jaredpar
Copy link
Member Author

FYI: updated the repro instructions. The perf is bad enough that I need to move back to the classic SDK for now. Saved the current state though in repro/fsharp-intellisense-perf branch

@cartermp
Copy link
Contributor

cartermp commented Jan 19, 2018 •
edited
Loading

Yeah, stuff seems to be needlessly recomputed here and it's not just the delay in the editor:

recompute

Note that successive . gives a spike in CPU usage even though nowhere near enough memory is being used to force things out of caches.

Also noticed a bug where SelectionTracker is being favored over _selectionTracker in completion, but that's unrelated to this.

jaredpar added a commit to VsVim/VsVim that referenced this issue Jan 19, 2018
@jaredpar
Move back to classic SDK
ec3b443 
The move to the new SDK uncovered a bug in the F# language services.
It's fairly impactful to typing so temporarily reverting back to the
classic SDK until it's addressed.

dotnet/fsharp#4209
@jaredpar jaredpar mentioned this issue Jan 19, 2018
Merged
@jaredpar
Copy link
Member Author

Once I moved back to the old SDK the issue went away. Only been a few minutes but everything seems to be behaving as I would expect.

@forki
Copy link
Contributor

forki commented Jan 19, 2018

I can repro. old sdk fast - new sdk branch slow

jaredpar added a commit to VsVim/VsVim that referenced this issue Jan 19, 2018
@jaredpar
Move back to classic SDK
5d66be0 
The move to the new SDK uncovered a bug in the F# language services.
It's fairly impactful to typing so temporarily reverting back to the
classic SDK until it's addressed.

dotnet/fsharp#4209
@cartermp cartermp added the Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code. label Jan 20, 2018
@cartermp
Copy link
Contributor

Labeling as Now since this is clearly a blocker for anyone who has a similar situation as @jaredpar

@dhwed
Copy link
Contributor

dhwed commented Jan 24, 2018

I thought that I fixed this with #4121 - is that change included in VS 15.6 preview 2?

@cartermp
Copy link
Contributor

cartermp commented Jan 24, 2018 via email

@jaredpar
Copy link
Member Author

@cartermp is the fix already checked into VS? I can check latest internal build of preview3 to see if it's fixed there for my case.

@cartermp
Copy link
Contributor

Yep, insertion should be complete soon

@jaredpar
Copy link
Member Author

What build should have it? Happy to try and validate this fixed it if I know what build to look for.

@cartermp
Copy link
Contributor

cartermp commented Jan 25, 2018 •
edited
Loading

Candidate build was d15.6/27323.02 - @brettfo is that still accurate?

@jaredpar
Copy link
Member Author

I just tested out VsVim on new SDK in 27319.1.d15.6. The perf problem appears to be fixed there. Intellisense has the performance I would expect.

@forki
Copy link
Contributor

forki commented Jan 25, 2018 via email

@cartermp
Copy link
Contributor

Excellent! Glad to see that @dhwed's changes here seem to have done the trick. Closing for now.

@cartermp cartermp added this to the 15.6 milestone Jan 25, 2018
@KevinRansom
Copy link
Member

My favourite phrase ever "has the performance I would expect.". Accurate, without giving away any details. :-)

@btrepp
Copy link

btrepp commented Jan 31, 2018 •
edited
Loading

Is there any way I can test/use this fix?. Or an ETA when it will be accessible?. I've tried the nightly VSIX in 15.5.6 and performance is still slow for me ( though I get a worse performance than ~8 seconds)

@cartermp
Copy link
Contributor

@btrepp Are you referring specifically to IntelliSense? 15.6 Preview 3 is the one that has the fix that addresses this specific issue.

@btrepp
Copy link

btrepp commented Feb 2, 2018 •
edited
Loading

Unfortunately preview 3 the IDE crashes on load of my solution :(.
#4300

@cartermp cartermp changed the title Intellisense taking ~8 seconds on new SDK Completion list takes far too long even after an initial list is generated May 8, 2018
@cartermp cartermp reopened this May 8, 2018
@buybackoff
Copy link
Contributor

@cartermp how I can help diagnose what's wrong? Should I try nightly build?

@cartermp
Copy link
Contributor

cartermp commented May 8, 2018

It's doubtful that a nightly has fixed this, but it's worth a shot. I think we'll just need to debug it and see what's going on, possibly running a trace. I can consistently reproduce your issue (every time I hit ., CPU spikes up to 60% and it takes multiple seconds to see completion). No inkling what the root cause is right now. Are there other places where completion consistently takes 8+ seconds, or is this the only one you've noticed so far with 15.7?

@buybackoff
Copy link
Contributor

buybackoff commented May 8, 2018 •
edited
Loading

Everywhere in the project where I'm trying so far. Removing netstandard2.0 target doesn't help. I could see by coloring that some heavy calcs are going one in background, even the gif above shows that this is white for 5-6 secs then it turns yellow and only after that the intellisense window is shown.

@davkean
Copy link
Member

davkean commented May 10, 2018

Hey folks, sorry you are running into issues. Can I ask you do something?

  • Open a new instance of Visual Studio
  • Send Help > Feedback -> Report a Problem -> Report new Problem
  • Enter title/description saying you running into perf issues
  • Click Next, under Attachments choose Record and choose the other instance
  • Click Start recording
  • Go back to the other insatnce, and repro the problem
  • Click Stop Recording
  • Send on the Report and link the developer community item when it gets sent via email

This will give us traces that help us figure out the root cause.

@davkean
Copy link
Member

davkean commented May 10, 2018

Nevermind, strike that - I missed we had a 100% repro on our side. If you are hitting other performance issues in the future in other areas, follow the above steps.

@cartermp
Copy link
Contributor

cartermp commented May 11, 2018 •
edited
Loading

Just as an update, with Spreads under the microscope, we identified a hot spot where things are being needlessly recomputed deep in the language service causing a 5-8+ second delay every time. When we profile Spreads converted back to old-style SDK, that hot spot is instant, and completion performance is what we would expect.

Clearly, something is not being wired up at a top level, but we're not sure what it is yet. More digging is underway...

@TIHan
Copy link
Contributor

TIHan commented May 12, 2018

I believe this should resolve it: #4889

This solves the issues in Area51 and Spreads that I managed to reproduce.

@buybackoff
Copy link
Contributor

buybackoff commented May 12, 2018 via email

@cartermp
Copy link
Contributor

This should be available in nightlies once merged, yes.

@TIHan
Copy link
Contributor

TIHan commented May 14, 2018 •
edited
Loading

@0x53A @buybackoff
I have to be a little more accurate here. I think when we get this fix out it should fix a lot of problems and you will see better results.

However, both of your projects still have a problem: multi-targeting. I did some testing today and multi-targeted projects will cause invalidation and rebuild chunks of the world. I don't know how complicated it is to fix, but I have a good understanding now of what's happening in our tooling to probably start trying to fix it.

From your projects, when using the fix here: #4889, completions and intellisense tend to be faster on immediate use. When you start using a bit more, you will notice it will take longer sometimes; that's due to invalidation from multi-targeting.

Removing multi-targeting and just doing one target, you shouldn't have any issues once the fix is in place.

@buybackoff
Copy link
Contributor

@TIHan I could live with only .NET Standard 2.0 and have started to remove full .NET Framework target in an active branch. .NET 4.6.1, which supports standard 2.0, is old enough, and I have had needless pain with supporting 4.5.x. So if you have to choose between an ideal solution vs quick fix then please ship it ASAP for a single target.

@buybackoff buybackoff mentioned this issue May 15, 2018
Closed
@buybackoff
Copy link
Contributor

Latest nightly works approximately as fast as Rider for autocomplete. Thanks!

@davkean
Copy link
Member

davkean commented May 15, 2018

@buybackoff Thanks for the followup!

@cartermp
Copy link
Contributor

I'll close this out and open a separate issue about perf for multitargeting, so that there isn't a multi-page issue to find out the current state of things.

@cartermp cartermp modified the milestones: 15.6, 15.7 May 15, 2018
@cartermp
Copy link
Contributor

Modified milestone to reflect that we're attempting to get this into a servicing branch.

@buybackoff
Copy link
Contributor

buybackoff commented May 25, 2018 •
edited
Loading

Just FYI, IntelliSense was dead completely in nightly (May 24 latest update at least, found y'day after rebooting and closing VS the first time after in started working) and VS 15.7.2 (or probably after updating VS). It was like Notepad.exe - just text. With May 25 nightly syntax coloring, tooltips and red squiggles work, but no autocomplete (at all, tested many minutes). Rider works. Same project, same file.

@cartermp
Copy link
Contributor

@buybackoff Not sure how this is on your machine, but with latest master of Spreads the solution won't compile, and I noticed that anything involving this in SortedMap will fail due to its inherited type (ContainerSeries<'K, 'V, SortedMapCursor<'K, 'V>>) being defined in a project which doesn't compile. Various other things seem to not work due to this.

Example of what's unresolved:

image

I do get IntelliSense, just not for things which are defined elsewhere (and failing to build):

image

This is with the May 25th nightlies atop 15.7.2. Is this consistent with what you're noticing?

@buybackoff
Copy link
Contributor

buybackoff commented May 25, 2018 via email

@cartermp
Copy link
Contributor

cartermp commented May 26, 2018 •
edited
Loading

@buybackoff I believe 15.7.2 contains one of the fixes, but not the multitargeting one. Since Spreads.sln is still using multitargeting, it's likely to be slow. If you can afford to retarget only to netstandard2.0, then things should remain fast enough such that the typechecker is the bottleneck again.

That said, the experience noted above is venturing into undefined behavior. Completion on this when the inheriting from something that isn't resolved currently doesn't exist as far as I can tell. It's arguable that failing to resolve at an inheritance context shouldn't affect other areas (e.g., the dictionary parameter to the default constructor of SortedMap being seen as unused), but I believe this has more to do with the compiler than with IDE tooling. I could be wrong, but there are various places where error recovery of the compiler prevents the tools from doing the right thing, and this looks like one of them.

The only true solution to that is to rewrite the parser to focus on error recovery (like the Roslyn team did for C#), but this is a large undertaking that is currently not as important as other performance issues, such as high memory usage and paying off years of technical debt w.r.t GC pause times under various scenarios. We get bit by it as well and want to do this, but the current work list of things we must do w.r.t performance and technical debt is quite large.

@buybackoff
Copy link
Contributor

How to easily install a concrete nightly version that worked several days ago? I have retargeted F# to only netstandard2.0 and everything worked very well with the nightly after the first fix (without multitargeting), but stopped working after restarting VS and new nightly automatically installed. This definitely not related to Spreads, a standalone Deedle (that targets .NET 4.0) doesn't have autocompletion even on List members.

The only change I did in additional to VS restart (and the reason to restart) is updating R# to 2018.1.1. But suspending it doesn't change anything in F#.

@cartermp
Copy link
Contributor

cartermp commented May 26, 2018 via email

@cartermp
Copy link
Contributor

@buybackoff Do you have a repro for what you mentioned here?

This definitely not related to Spreads, a standalone Deedle (that targets .NET 4.0) doesn't have autocompletion even on List members.

Various files seem fine for me with Deedle.sln:

image

image

This is with the may 25th nightlies installed into 15.7.2.

@buybackoff
Copy link
Contributor

@cartermp that's weird but now I have it working even in Spreads and seems like understand the reason: my copy of Deedle was not built and I disabled the automatic build of Spreads's F# project (Collections) while testing its dependency (Core) to save time on build while making many small changes in the Core project. If I clean the F# project in Spreads I could reproduce that there is no autocomplete.

Yet definitely I was running tests that used the F#-produced dll, but it was not rebuilt while it dependency was changing. When I delete obj folder I could run tests with the F#-produced dll and at the same time IDE for F# is not working. It starts working after rebuilding the F# project. So maybe at some point the content of the obj folder in F# project became incompatible with it's dependency and autocomplete failed. Or something else.

@cartermp
Copy link
Contributor

There are some known issues around /obj in the new project system that have to be worked through. So if deleting that folder made things better, then it sounds like you ran into one of those. They're difficult to reproduce, unfortunately, so I don't know of a repro off-hand that you can observe on any machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-LangService-API Impact-High (Internal MS Team use only) Describes an issue with extreme impact on existing code.
Projects
None yet
Milestone
15.7
Development

No branches or pull requests