dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.02k stars 4.03k forks source link

Unexpected slowness with javascript #24503

Closed velociwabbit closed 3 years ago

velociwabbit commented 6 years ago

Version Used: 2017 latest release 4.7.02556 Steps to Reproduce:

  1. Virtually every single javascript.js file has this problem
  2. scroll down to the bottom of the file open then ctrl f to open the find window
  3. the system hangs for 20 seconds or more

Expected Behavior: that it work without delay Actual Behavior: that it reminds me of software when balmer was in charge

I have reviewed this issue online and even though i am not using c++ or csharp I cannot turn of roslyn... then the app has the unmitigated cheek to even tell me that roslyn is slowing things down. And then the button to turn it off is disabled...

image

jmarolf commented 6 years ago

@velociwabbit javascript and typescript use roslyn for most of their editor functionality so this UI delay is likely coming from the javascript language service itself and being reported as roslyn. Please follow these instructions on how to get a trace so we can diagnose this, thanks!

sharwell commented 6 years ago

Hi @velociwabbit,

@dpoeschl and I worked together on the instructions posted by @jmarolf. This set of instructions has been extremely successful at helping users report problems in ways that allow us to get them fixed efficiently. After you complete the steps, please comment back here with a link to the feedback item you added on Developer Community (thousands of issues get filed and I want to make sure we can find the one(s) specifically created for this issue).

If you encounter problems or are unable to post the feedback, please let us know and we can find another way to get the information needed to resolve this.

Thanks!

velociwabbit commented 6 years ago

While I understand politely admonishing me for my attempt at levity in the creation of this incident (fair enough but it is better than flaming ) I must tell you that asking me in essence to be more specific in my incident is in some ways (assuming that someone at MS is working on this ) tantamount to the pot calling the kettle black... here is a great example. This link https://github.com/dotnet/roslyn/wiki/Reporting-Visual-Studio-crashes-and-performance-issues#performance-issues has an instruction to use the tool for reporting incidents in visual studio has this specific instruction "open the Report a Problem tool". Yet when i go to tools there is not "Report a Problem" option... nor is there one on any other menu.

If I did not know any better I would think that the same doc author recently changed jobs and used to or is now working at aws :)

If you want me to go the extra mile by helping to debug this problem it would help to know that specificity is the norm not just for the poor schmuck user (yours truly )

Come guys you are better than aws ... right ?

tempcap

tannergooding commented 6 years ago

The "Report a Problem" tool can be found via the help menu: image

The the little person icon next to the "Quick Launch" text box: image

Or via the "Quick Launch" text box: image

velociwabbit commented 6 years ago

This is getting to the point of being comical in and of itself.

After several minutes of menu and submenu perusal i finally did find the Help->Send Feedback->Report a Problem option only to be asked to login to create a new problem.

The problem is that i am already logged in... and this picture shows this apparent contradiction. EIther this is a bug or I am not aware of the secret society that i need to subscribe to. capture2

velociwabbit commented 6 years ago

It appears that we have a Goedel-bug (i just made that up... but i think it is both specific and useful in this situation) In other words the bug is manafest by self referencing.

It also makes the rest of the bugs in my VS instance heisen-bugs since the act of trying to address them is made impossible by the fact that the system itself can never see either the bug itself or a dump of the system when the bug occurs at the same time (kind of).

What i am hoping for is some spooky action at a distance (that would be from seattle in this particular instance) :)

sharwell commented 6 years ago

@velociwabbit Send me an email and I can get you in touch with people who can help. You can find my email by opening one of my pull requests, e.g. #24189, and adding .patch to the end of the address. This is a semi-documented GitHub feature derived from this example API response and mentioned in this blog post.

nikolami commented 6 years ago

@velociwabbit, thank you for trying to report a problem and for following up with us when that failed. I apologize for the issues you're seeing. I'm a developer on the feedback team.

To resolve the "cannot use Report Problem" issue, once you start Report Problem, please click on the link that reads "Re-enter your credentials." It's in the top part of the window. I can see part of it in your last screenshot above.

About re-entering credentials in Report Problem tool: In VS, your login carries identity information (e.g. anonymous user ID) and personal settings (e.g. whether you use the dark or light theme.) When reporting a problem, we ask VS for an authentication token. When we get one, however, it's sometimes only good for personal settings. Thus it's OK in VS (logged in for the purpose of syncing your theme) however does not work for us to find your info for the purposes of reporting a problem. That's why we ask to "Re-enter your credentials." Having this two-token decision allows for better security and less login prompts in VS since the personalization token can be set to expire slower than the identity token. "Sign in too often" in VS used to be a major complaint. More info here: https://blogs.msdn.microsoft.com/visualstudio/2016/08/15/fewer-visual-studio-sign-in-prompts/

I opened three bugs in our private tracking system to fix these UX issues:

  1. The documentation about how to find report problem
  2. Make the "Re-enter your credentials" easier to spot
  3. Explain why re-entering credentials is needed

I also sent email to our team with the bugs and the link to this GitHub conversation. If you see any further issues, please let me know! Thanks again for being proactive.

velociwabbit commented 6 years ago

I would think that if someone was willing to take the time to report a problem that any security to stop that process would not be in the interest of MS or or vs. Why have security on reporting problems full stop?

IF you are worried about attacks certainly you could send the reports and the data to a filter of some sort so that they cannot disrupt any service.

As you might imagine after taking the time to try and figure out how to report a problem being questioned about ones credentials and therefore legitimacy to help make vs better is not really something that says thank you for following up with us and we are sorry that the product is not yet cooked! It says "you better be someone we value or you are wasting our time"

This javascript/roslyn problem is a very serious performance limitation and it has been happening for the last year. It can easily be reproduced by loading large javascript files into an editor and then removing all the training wheels syntax (such as ; at the end of each line) and doing other optimization for example complex ternary expressions instead of if then spaghetti or no brackets around single if statements. Stuff that makes javascript readable and easy to program (which is the opposite of lint /typescript enforcement.)

I understand that novice programmers who do not understand the nuances of javascript need training wheels but your syntax manager should not punish the advanced js programmer just because they are not morons.

CyrusNajmabadi commented 6 years ago

@velociwabbit Can you share a repro that demonstrates the problem you're seeing when editing JS/TS.

but your syntax manager should not punish the advanced js programmer

Having written the JS/TS syntactic infrastructure, i can assure that it was never designed to do any of that :)

It sounds likely that you're running into some issue that's causing unintended pathological behavior. If you can provide a repro that would go a long way toward address the issue.

CyrusNajmabadi commented 6 years ago

It can easily be reproduced by loading large javascript files into an editor and then removing all the training wheels syntax (such as ; at the end of each line) and doing other optimization for example complex ternary expressions instead of if then spaghetti or no brackets around single if statements

I can't repro this. The current JS i'm working with is several hundred kloc. I removed all the optional semicolons, and i even added a lot of complex ternary expressions to no avail.

--

There is something that could be causing this. Namely that some part of typescript type-inference are exponential in cost. This isn't uncommon in type inference systems (C#, C++, haskell, swift, and others all suffer from this). It's possible that you might have some type inference case that is hitting this and slowing things down. With a repro we might be able to track things down.

Also note: TS/JS runs out of https://github.com/Microsoft/TypeScript . I'm tagging @DanielRosenwasser so he's aware of this conversation going on here.

DanielRosenwasser commented 6 years ago

Sounds bad. Our team (TypeScript/JavaScript tooling at Microsoft) would be willing to sign an NDA if you'd be willing to share your project with us.

CC @billti

velociwabbit commented 6 years ago

great i am in contact with another person and i will share your offer. and have him copy you on our correspondance

On Tue, Jan 30, 2018 at 9:39 PM, Daniel Rosenwasser < notifications@github.com> wrote:

Sounds bad. Our team (TypeScript/JavaScript at Microsoft) would be willing to sign an NDA if you'd be willing to share your project with us.

CC @billti https://github.com/billti

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dotnet/roslyn/issues/24503#issuecomment-361829637, or mute the thread https://github.com/notifications/unsubscribe-auth/AI7J7brxrR7cnVeySquW9A2XR7bc5Yiuks5tP_yagaJpZM4RwC_T .

velociwabbit commented 6 years ago

i have attempted to disable all typescript processing and all other types of editorial interference in javascript, json, css, html in the options -> text editor ->html... etc sections so if vs is disabling them incorrectly then you are correct in your assessment (about the inference systems).

I also do several other non conventional formatting choices that must be giving the editor trouble. GIven that our eyes are placed on a horizontal plane (which is why we have faces that do not look like something out of family guy or southpark ) , and that most monitors are 1920,1080 or 4k x~2k and the that i have never printed out my code since windows XT I try to use all of the monitor screen by keeping whole javascript statements on one line (as much as is logically sensible) and heavily indenting (sometimes up to half the screen).

I realize that the python approach (where every space is precious, every space is great, if we waste our spaces god gets quite irate! ) is the way most people prefer operating. No doubt because they feel that their short term memory is better than their ability to read left to right and therefore they have no problem with 1 statement taking up the entire screen. I am not so fortunate (or so willing to be inefficient?). Perhaps most people are now inverting their screens to 1080 x 1920 (but i digress with my little rant about formatting)

I also try to minimize the number of files and folders for my app. I get this very rare (or not so rare) disorder which i think should be called "Directory Vertigo" which is manifest in a state of constant confusion where you spend most of your time trying to figure out which index.js is the correct one for your react redux / forms action creators or other self important and nearly useless stand alone files.

while i am coding and debugging my app i try to minimize the number of unforced errors by keeping as much code as possible consolidated into a few files so my feeble memory does not have to remember 6 levels of directories and 100 files with the same index.js name.

No doubt i am a pathologic case but if common sense ever prevails in software standards all code should begin to look like mine. (On the other hand we still use the qwerty keyboard )

velociwabbit commented 6 years ago

And now that i am finally able to vent my frustration at the brilliant choices lint and other self declared standards bodies have made about how to place code in files there is one more ahem enlightened? standard that mystifies me. (And no this comment does not belong here but it should be said somewhere and why not to the VS audience?)

Modularization is like candy ... too much of it and it will make you sick... way too much of it and you will get diabetes... way way to much and it is lethal.

It is now to the point where the average number of lines of active code is about the same as the number of files in a repo. I am certain of this because i am following the official donald trump fake vs real news standards!

The point of a file is to contain a lot of useful and related information.

The point of a folder is to contain many useful files.

Those geniuses at sun who created marimba and java at the same breakfast decided that directories with files made of classes are a great way to enhance the usability of software... kind of like the first person who ever moved to out of LA (because it was a great big freeway) to San Jose (which is a sleepy little town).

I have a great idea why not place every word in the english language in its own repo so that all we have to do is to import, include, require, each word in each module that we create. That way if there is a speling mistake that repo with the one whole word that is misspelled can be modified and no one else will have to modify their import statements :)

Satire aside there is a real benefit to keeping code in one location and with one developer... there was a book about it called the mythical man month... Most smart companies only put one or two developers on a project because any more just slows down the process and produces buggy, top heavy code.

Repo madness encourage collaboration on code which is great in theory but ultimately depends upon the weakest link for quality.

As an exercise in futility try to consolidate all the import statements from any modern app into one file...

I just hope that none of this code is running nuclear reactors

sharwell commented 6 years ago

@velociwabbit Keep in mind that questions about code style are not intended to get you to change code style. We want the IDE to work with whatever style you are comfortable with. The questions were simply intended to help create a scenario as close to yours as possible so we can observe (and then fix) the scenario matching your current experience.

velociwabbit commented 6 years ago

Once again I apologize for the inappropriateness of my rant... but if there are any others that agree with me perhaps we can form a support group (cheaper than therapy!)

sharwell commented 6 years ago

@velociwabbit The rant doesn't bother me. It sounds like you spend a good portion of your day working with VS and it's natural to feel passionately about something you have so much experience with. Just remember that other readers can't see the messages you sent privately (or their responses), so they won't be aware of any steps we take to help you unless you comment about that part too. 😄

velociwabbit commented 6 years ago

To be honest this is the first time in ages i have tried to work with MS on anything ... and i am very very impressed with both the turnaround time and degree of professionalism. (Two statements that would have been heresy 10 years ago. ). And directly because of this i am going to try Azure for my next app... I am using aws and frankly the aws documentation appears to have been written by someone who just graduated from the devry school of computin and carburetor repair

nikolami commented 6 years ago

re: report problem auth: @velociwabbit wrote: ...if someone was willing to take the time to report a problem that any security to stop that process would not be in the interest of MS or or vs. Why have security on reporting problems full stop?

Very good question. We just discussed it again on the Feedback team. For customers submitting sensitive information (crash dumps, logs) authentication helps ensure their information is protected. For example, we don't want someone to send us private information without being authenticated, then someone else impersonating them, and potentially gaining access to their data. Example: think a VS enterprise customer working at company X on company X's private project sending us a source file or a memory dump after a crash. My personal opinion: better be paranoid and ask for login when sensitive data is (maybe) involved.

Regarding the "logged into VS but not into Report Problem" - I added a task to our private planning docs to chat with the auth team. We'll discuss if there's a way to keep Report problem secure without asking for login if VS is already showing as logged in all cases. Currently, we only migrate the login info from VS if we find the shorter-expiry auth token. Thanks for the feedback again!

velociwabbit commented 6 years ago

Another alternative is to inform the user that the report submittal is not secure and if they would like to ensure security / authentication that they would have the option of logging in another time... that way you dont turn every user into a larger ponderous corporate user.

optimizing for the extreme guarantees alienation of the majority ... (not to make this a political statement of sorts :) )

On Wed, Jan 31, 2018 at 2:30 PM, Nikola notifications@github.com wrote:

re: report problem auth: @velociwabbit https://github.com/velociwabbit wrote: ...if someone was willing to take the time to report a problem that any security to stop that process would not be in the interest of MS or or vs. Why have security on reporting problems full stop?

Very good question. We just discussed it again on the Feedback team. For customers submitting sensitive information (crash dumps, logs) authentication helps ensure their information is protected. For example, we don't want someone to send us private information without being authenticated, then someone else impersonating them, and potentially gaining access to their data. Example: think a VS enterprise customer working at company X on company X's private project sending us a source file or a memory dump after a crash. My personal opinion: better be paranoid and ask for login when sensitive data is (maybe) involved.

Regarding the "logged into VS but not into Report Problem" - I added a task to our private planning docs to chat with the auth team. We'll discuss if there's a way to keep Report problem secure without asking for login if VS is already showing as logged in.

Hope that helps and thanks for the feedback again!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dotnet/roslyn/issues/24503#issuecomment-362092922, or mute the thread https://github.com/notifications/unsubscribe-auth/AI7J7WIQbYtGFwdwYlzff3ZwAbiv7L50ks5tQOl4gaJpZM4RwC_T .

jmarolf commented 6 years ago

A similar issue was also reported here: https://developercommunity.visualstudio.com/content/problem/191828/editing-typescript-files-roslyn-performance-issues.html

velociwabbit commented 6 years ago

still waiting for the nda so i can send someone my app.

CyrusNajmabadi commented 6 years ago

Ping @DanielRosenwasser

sharwell commented 6 years ago

@velociwabbit There is an internal discussion going on about how to proceed. The rules recently changed so the team that contacted you is making sure they have the latest information from legal.

billti commented 6 years ago

If you/your company has NDA requirements, then you need to provide the NDA document outlining those requirements for us to agree to, sign, and return. We're happy to do so (assuming the requirements are reasonable) once we receive it.

You already have Sam's email, you can include me also via billti at microsoft dot com. Thanks.

nikolami commented 6 years ago

@velociwabbit - to avoid derailing the github discussion from the original issue, please feel free to ping me my github nickname at microsoft dot com. I'm open to discussing this over email (or if you report a new problem in the developer community VS forums :)

gundermanc commented 6 years ago

@sharwell Has TextMate been ruled out as a potential root cause? It is used for the colorization of TypeScript and JS. If anyone has ETW traces I'd be happy to take a look.

sharwell commented 6 years ago

I've seen some performance traces recently related to JavaScript, but none yet (knowingly) associated with this particular bug report.

I do have a repro file, but so far I wasn't able to reproduce the issue with it. I'll be following up today.

gundermanc commented 6 years ago

Ok, if you see Microsoft.VisualStudio.TMParser on the stack as an offender, send it my direction.

ghost commented 3 years ago

Closing this issue as we've seen no reply to the request for more information. If you are able to get the requested information, please add it to the issue and we will retriage it.