TypeStrong / atom-typescript

The only TypeScript package you will ever need
https://atom.io/packages/atom-typescript
MIT License
1.13k stars 205 forks source link

atom typescript slows down the editor in large files #1554

Closed aminya closed 3 years ago

aminya commented 4 years ago

Try openning a big file and edit it and move the cursor around. Each cursor movement takes some time. Like: https://github.com/atom/atom/blob/master/src/package.js

The only cursor trackers I can think of in atom-typecsript is the SemanticsView. This is strange because my atom-ide-outline does not slow down the editor (although I have written atom-ide-outline using pure DOM, so it is expected to be faster than etch).

If you know any other, you might have to optimize the code.

lierdakil commented 4 years ago

There's also an occurrence tracker which binds to cursor movement. That is, the thing that highlights all occurrences of the identifier under text cursor.

lierdakil commented 4 years ago

Also, I'm not seeing the issue: https://youtu.be/TRxOH-Y28mU

aminya commented 4 years ago

I am going to do more investigation to see which plugin is causing this!

aminya commented 4 years ago

I found an easy way to measure things. I noticed that these functions are taking most of the time: image

I disabled the checkAllOption in https://github.com/TypeStrong/atom-typescript/pull/1555

However, geterr is slow, and it runs on every change I make in the editor. For example, this is the result of pressing the space bar:

image

lierdakil commented 4 years ago

Getting response for geterr request can easily take half a second, but it is an asynchronous request to a node process, so it does not block the main UI thread.

Just use the developer tools profiling tab to figure out what's going on. Measuring how long async functions take to resolve is not a meaningful metric.

aminya commented 4 years ago

@lierdakil Here is the profile of me just typing a few things: Profile-20200725T074758.zip

I am seeing something very strange! I do not have atom-beautify installed, but it is shown in this profile.

image

Getting response for geterr request can easily take half a second, but it is an asynchronous request to a node process, so it does not block the main UI thread.

Javascript is one-threaded in the end and async does not help much for intensive tasks. If the issue is just an external server response then async helps. I have not checked the geterr code, so I am not sure about it.

Off-topic: If you want to get real multi-threaded experience, you should use worker_threads or web workers. Last I tried worker_threads was not supported in the V8 engine of Atom yet. I haven't checked web workers though. Check this article: https://blog.logrocket.com/node-js-multithreading-what-are-worker-threads-and-why-do-they-matter-48ab102f8b10/

aminya commented 4 years ago

Ignore the previous profile. Atom beautify had not been uninstalled yet. It was just the wrong name for the package that is using loadash.

Here is the correct profile: Profile-20200725T083003.zip

All of the hot ones are related to atom-typescript:

image

image

aminya commented 4 years ago

Even with merging #1556 With #1555 enabled, errorPusher gets slower when I save my files. Even when I am editing a single file and I am not checking all files again! 600ms in each keystroke is not acceptable. image

lierdakil commented 4 years ago

According to the profile, the vast majority of the time gets spent in Atom's bowels, guessing which grammar should be used for a file path. Okay, we can combat that through memorization.

lierdakil commented 4 years ago

Since I'm still unsure how to repro, can you re-profile on v13.9.2 to see if anything changed? Thanks.

aminya commented 4 years ago

Nice, I can feel that it is much better now! I will give you more benchmarks later, but here you go for this basic profile of me typing. checkAll option is disabled. checkAllDisabled.zip

aminya commented 4 years ago

@lierdakil One other case that I have noticed takes for atom-typescript is when I open a big bundled file (like in dist folder), although I have gitignored this file.

Like in this case I opened a dist/bundle.js file and after that Atom was unresponsive for 30s or more. Annotation 2020-08-04 210159

This brings up an idea. Currently, Atom doesn't skip processing the files that are minified or are very large and stored in gitignored repositories.

I think we can create a feature in Atom to prevent any processing happen on these types of files. I have seen this behavior on other IDEs too.

lierdakil commented 4 years ago

Atom was unresponsive for 30s or more.

All the operations you list are fully async, so Atom gets frozen for an unrelated reason. Again, run a profile. Getting run time for async functions that wait 99% of time is not helpful at all.

aminya commented 3 years ago

For this, we need to implement some mechanism in the Atom itself. There is already largeFileMode, but we need to extend that to include long lines.

In VsCode, they skip the tokenization in large or minified files. That prevents any plugin to work and break VsCode. We should do a similar thing inside Atom. image

github-actions[bot] commented 3 years ago

This issue has been marked as stale because it did not have any activity for the last 90 days or more. Remove the stale label or comment or this will be closed in 14 days