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.

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

Package loading time #430

Closed
csholmq opened this issue Dec 16, 2015 · 8 comments
Closed

Package loading time #430

csholmq opened this issue Dec 16, 2015 · 8 comments

Comments

@csholmq
Copy link

csholmq commented Dec 16, 2015

image

How come this package takes more than half a second to load? Only takes 16 ms to activate.

@abe33
Copy link
Contributor

abe33 commented Dec 16, 2015

You know that these measures can be altered by other processes, right?

IE. I have 3 atom windows open at the time, here's the results (on a 3 years old iMac with no SSD):

Atom 1

Atom 2

Atom 3

As always when measuring performance, the smallest measure is always the one closest to the real data.
And as you can see in the screenshots above, it can grow easily by a 6x factor, maybe more depending on what your system is doing on the background. So it's not like we can do anything in that regard.

@csholmq
Copy link
Author

csholmq commented Dec 16, 2015

That is true! However, minimap constantly ends up on top when I reopen Atom. With no project/tab loading at startup, it only takes ~100 ms. So likely related to the number of tabs present.

@abe33
Copy link
Contributor

abe33 commented Dec 16, 2015

So likely related to the number of tabs present.

That would affect the activation time rather than the loading time (it's during the activation that we create a minimap for each editor). What would impact the loading time is if we were using serialized data in the package, but that's not the case. I guess babel compilation time might be the real culprit here.

@csholmq
Copy link
Author

csholmq commented Dec 16, 2015

So no requires that could be moved to activation instead?

@abe33
Copy link
Contributor

abe33 commented Dec 16, 2015

I'm not against it (even if it makes the code less clear as requires are spread all over the place), but I would like to back up the change with some metrics to be sure it has a perceivable impact. In theory, the gain on loading time would only be deported on activation time.

@csholmq
Copy link
Author

csholmq commented Dec 17, 2015

Issue atom/atom#2654 is a good thread to scroll through. Especially atom/atom#2654 (comment) and atom/atom#2654 (comment) which both talk about speedups when deferring requires.

If minimap activation would be possible to trig by e.g. initial scroll or perhaps pane focus, then any requires would only be fired when that particular tab is of interest. Especially for me who uses minimap-autohide which means I don't expect to see the plugin until I scroll.

@abe33
Copy link
Contributor

abe33 commented Dec 17, 2015

Issue atom/atom#2654 is a good thread to scroll through. Especially atom/atom#2654 (comment) and atom/atom#2654 (comment) which both talk about speedups when deferring requires.

Yeah, I know this thread, of course, but it was identified that most of the startup bloat at the time was due to node's findPath method that was hitting the file system to stat files and find the proper path. This was fixed since age.

Anyway, I'm now lazy loading the model and views on activation, but, I can't assert it gives significant improvement, given that, as I already said, startup time is highly dependent on what happens on the system at the same time.

For instance, using the current version, I have loading time from <5ms (not listed in timecop) to 300ms, with the lazy-load one, I pretty much get the same results, and I can't proves that the differences come from the change (what if I start Atom at a moment when the file system is not accessed by anything else).
Same thing for the activation time, depending on the condition, I got measures from ~40ms to 2s on the same project. With this much gap in the results any micro optimization is meaningless. Most of the bloat comes from external factor I can't control.

If minimap activation would be possible to trig by e.g. initial scroll or perhaps pane focus

That's a no go for me. Too much complexity for too little gain and no real data to back it up. As far as I can tell very few people are using minimap-autohide in comparison with minimap.

@abe33 abe33 closed this as completed in e0b1127 Dec 17, 2015
@csholmq
Copy link
Author

csholmq commented Dec 17, 2015

You have very valid points. And my empirical testing validates that indeed the time is shifted over to package activation with e0b1127.

For this package it may not matter much for the startup loading time whether it's in package loading or in activation. But I believe a lot of packages that depend on different types of trigs could benefit from this. Perhaps this wouldn't be an issue if the Atom window would properly render the window and tabs directly after the core packages had loaded. But that is another issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants