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

Packages preloading #14080

Merged
merged 16 commits into from Apr 3, 2017

Conversation

Projects
None yet
8 participants
@as-cii
Member

as-cii commented Mar 28, 2017

This pull request aims at reducing bundled packages loading and activation time even further by preloading them. During the snapshot phase, in fact, Atom will now scan all the core packages and load their main module, keymaps, menus, and settings. Later, at runtime, it will finish loading those packages by compiling their style sheets, calling activate on them, etc.

In addition, this pull request improves the way packages are resolved and, as a result, minimizes the I/O cost associated to it. Previously, in fact, because of the way the code was structured, we would get the list of package directories and constantly issue a resolvePackagePath for each of them. This, in turn, would unnecessarily verify again that each path was a directory, thus querying the file system and blocking the main thread. After these changes, we will perform that I/O just once and return a javascript object which contains all the information needed to load the associated package without extra cost.

On master, this is how loading and activating packages looks like:

screen shot 2017-03-28 at 12 10 41

screen shot 2017-03-28 at 12 09 41

This is how the same scenario looks like after the changes contained in this pull request:

screen shot 2017-03-28 at 12 05 44

screen shot 2017-03-28 at 12 07 39

You can notice how startup got ~ 90ms faster, and packages loading alone became almost instantaneous. I am expecting this to be even more noticeable on machines that have slow hard drives, where the cost of I/O is much more dramatic.

This marks the end of the first round of performance improvements that can be achieved thanks to snapshots; there are still many things we can improve both in the renderer process and in the browser process, especially related to the scheduling of the tasks that currently happen as part of startup. I believe Atom could be a lot smarter about that, but I am going to illustrate what and how we could make it so in a separate issue (that is going to supplant the now outdated #13253).

/cc: @atom/maintainers

@ungb

This comment has been minimized.

Show comment
Hide comment
@ungb

ungb Mar 31, 2017

Contributor

@as-cii. I didn't see any issues with core packages on mac, saw huge performance gains.

Will continue to test on windows and linux

Contributor

ungb commented Mar 31, 2017

@as-cii. I didn't see any issues with core packages on mac, saw huge performance gains.

Will continue to test on windows and linux

@ungb

This comment has been minimized.

Show comment
Hide comment
@ungb

ungb Mar 31, 2017

Contributor

I didn't see any issues on linux or windows. I tested this with a few different packages installed and tested a few core packages like markdown, autocomplete, etc. LGTM 🚀 it!

Windows:
Before:
image

After:

image

Contributor

ungb commented Mar 31, 2017

I didn't see any issues on linux or windows. I tested this with a few different packages installed and tested a few core packages like markdown, autocomplete, etc. LGTM 🚀 it!

Windows:
Before:
image

After:

image

@50Wliu 50Wliu removed the in progress label Mar 31, 2017

@as-cii as-cii merged commit cef72fd into master Apr 3, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@as-cii as-cii deleted the as-preload-packages branch Apr 3, 2017

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Apr 3, 2017

Contributor

👏

Contributor

maxbrunsfeld commented Apr 3, 2017

👏

@steakknife

This comment has been minimized.

Show comment
Hide comment
@steakknife

steakknife Apr 21, 2017

Contributor

Packages cannot be preloaded until updateProcessEnv runs.

Contributor

steakknife commented on de47a26 Apr 21, 2017

Packages cannot be preloaded until updateProcessEnv runs.

This comment has been minimized.

Show comment
Hide comment
@as-cii

as-cii Apr 24, 2017

Member

Are you hitting any issue because of this? Preloading only affects core packages, which at runtime are then disabled in case a dev version of the package is found or the user had disabled the package in her configuration file.

Member

as-cii replied Apr 24, 2017

Are you hitting any issue because of this? Preloading only affects core packages, which at runtime are then disabled in case a dev version of the package is found or the user had disabled the package in her configuration file.

@whmountains

This comment has been minimized.

Show comment
Hide comment
@whmountains

whmountains Apr 22, 2017

Can you also allow third-party packages to be preloaded and shapshoted? Making a package "snapshot-friendly" would be like one step beyond making it lazy-load everything.

whmountains commented Apr 22, 2017

Can you also allow third-party packages to be preloaded and shapshoted? Making a package "snapshot-friendly" would be like one step beyond making it lazy-load everything.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Apr 24, 2017

Contributor

@whmountains It's something we could explore in the next round of startup improvements. For now we've moved to editor performance for a while but we will cycle back.

Contributor

nathansobo commented Apr 24, 2017

@whmountains It's something we could explore in the next round of startup improvements. For now we've moved to editor performance for a while but we will cycle back.

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