-
Notifications
You must be signed in to change notification settings - Fork 17.3k
Startup Snapshot #13916
Startup Snapshot #13916
Conversation
@as-cii I got this error when I built b2983f6. I did not get it when I was testing 7caeb3d. Atom: 1.17.0-dev-b2983f6 x64 Stack TraceFailed to activate the pigments package
Commands
Non-Core Packages
|
@as-cii If I do open folder I see a null/coffee folder. Creating files here reveal some files in the tree-view. Edit: I can't reproduce this and I was doing things I haven't done before so maybe it is just me misunderstanding things. Let's see if I can figure out why I saw these files or not. |
Thank you so much for testing this, @Ben3eeE! ❤️ I have submitted a pull request on the pigments package that fixes the issue you encountered; they were monkey patching some native modules assuming to find them in As for the It's worth noting that @Ben3eeE was also able to reproduce a hard crash when booting Atom, so I am going to spawn a Windows VM and investigate the problem. Based on the crash dumps he reported, it looks like the error is coming from pathwatcher and looks very similar to what we supposedly fixed in atom/node-pathwatcher#118. There are probably other race conditions and now that Atom loads faster they might have become more evident. |
Yeah... I'll see about the null folder. I'm completely clueless about where it came from and how to get it to appear. Maybe it was something that did belong to that project but like why? I am building latest master now to see if the crashes reproduce there but I likely won't be able to test that build on the same computer until tomorrow. The crashes were much more frequent outside of safe mode but I crashed when launching safe mode also. I'll also test and compare launch speed with master now that I got a feel for this. Most of my Atom restarts are because of installing a different version to test regressions so it's hard to quantify speedup for me. I did find one or two other regressions when testing this that might be from the jQuery removal. I'll open issues for those and mention @as-cii in them. |
@as-cii I was able to reproduce the crashes on master so it's not a regression in this PR. It's something that should be looked at but not something that should block this PR. I was unable to reproduce the crashes on my Windows 10 machine. The project that I load here is on a network share so maybe Windows has problems watching paths on a network drive. As for startup speed I see a 2 second improvement on both computers I tested. Loading the network share folder on my Windows 7 machine took about 8 seconds on master and 6 seconds on this branch. Loading a local folder on my Windows 10 machine took about 6 seconds on master and 4 seconds on this branch. Timed using my handy stopwatch, from clicking the icon to Atom being fully rendered and syntax highlighted. |
Atom: 1.17.0-dev-b2983f6 x64 Stack TraceUncaught TypeError: Cannot read property 'src' of undefined
Commands
Non-Core Packages
|
Thanks for the feedback, @Ben3eeE. It's so great to hear the improvement on Windows is so dramatic! ✨ I have been working on other improvements that take advantage of the snapshot which should minimize those times even further. As soon as this gets merged, I will open a pull request for those improvements too. atom/autocomplete-html#53 should fix the error you reported (as well as removing an extra fs call 🐎): I have pushed the fix onto this branch, so I think this is ready for another test drive! 👍 |
Building now ✨ Really great work here @as-cii can't wait for the other improvements ⚡ 💯 |
4c7800d
to
a8b885c
Compare
@as-cii Can confirm that the HTML exception is gone. Will rebuild again and test css as well just to be sure. |
Okay, so the failure on AppVeyor seems the same we are observing on master right now, so this pull request shouldn't have affected it. A huge shoutout goes to @Ben3eeE for testing this out and to @ungb for helping me test packages during the jQuery removal. ✨ Also, thank you so much @maxbrunsfeld, @nathansobo and @damieng for helping me out with #13254. ⚡️ I will go ahead and 🚢 this so that everyone on the team can try it out and catch as many regressions as possible before releasing it to beta next month. |
👏👏👏 Bravo |
Amazing! |
For a long time in Atom we have been wanting to have more control over startup time and, as part of this effort, back in November of last year we created an in-depth issue that illustrated where time was (and is) spent during the initialization of an Atom window. What we realized at the time was that most of the operations happening during startup were redundant and that we could use the information we had at build-time to minimize their cost.
V8 snapshots
V8 snapshots allow Electron applications to execute an arbitrary JavaScript file and output a binary file containing a heap with all the data that's left in memory after running a GC at the end of the provided script.
This suits the startup scenario described above perfectly, because we could use it to eagerly perform work when running
script/build
and just expose the computed data to Atom when it is opened.The tricky part of using snapshots is that the code is executed in a bare V8 context, meaning that we have no access to Node/Electron features nor to DOM APIs and we can only run plain JavaScript code without having access to native modules either. So, in order to circumvent this issue, we decided to defer usages of forbidden APIs until runtime so that all the other computation could still happen as part of the snapshot. As a result, we set out to create a tool that automates deferring the usage such features in a way that didn't sacrifice code readability: electron-link.
electron-link
electron-link is node module that takes a JavaScript file as the entry point and a list of modules that need to be required lazily. Then, starting from the main module, it traverses the entire require graph, replacing all the forbidden requires in each file with a function that will be executed later at runtime. The output of electron-link is a file that can be passed to the
mksnapshot
utility to eventually generate the snapshot blob for use in Electron.electron-link can also determine whether a module can be snapshotted or not. For instance, the following code can be snapshotted:
And the resulting code would look similar to the following snippet:
Conversely, when trying to transform the following code, electron-link will throw an error because it is using a forbidden module at require-time:
Improving
require
timeWhile this pull request lays the groundwork for using snapshots for all the concerns described in #13253, it only speeds up the time we were previously spending in
require
calls. The reason for beginning with just this one feature is that it will allow us to verify the feasibility of snapshots in production and to ship new performance improvements incrementally.The following is a typical chart of Atom startup time:
Please, note that these benchmarks profile everything from startup till the end of packages activation. There is other stuff going on after activating packages but it depends on asynchronous I/O and the scheduling of idle callbacks, so it adds noise to the measurements.
On average it takes
~ 0.9s - 1.0s
to load a stock Atom window and activate its packages when no editor is open and the tree-view is closed. A lot of it is spent in requiring modules and executing them to create classes or define objects. This is how the same chart looks like after this pull request:You can notice how the calls to
require
have almost disappeared (some still need to take place for Node core modules) and how the total time has went down to~ 0.7s - 0.8s
improving startup time by ~ 15-20%.Another scenario worth of comparison is loading an Atom window with the tree-view opened. This is how it looks like on master:
You can see it was
~ 100ms - 120ms
slower and, in part, this is due to loading the package's modules. With the snapshot, on the other hand, we already paid that cost upfront duringscript/build
and therefore the chart looks almost the same in terms of time (we still incur some overhead due to accessing the disk synchronously in the tree-view, but that's another story):This kind of improvement should apply to all the bundled packages because they are now required during the snapshotting phase.
Caveats
Please, note that this pull request disables the asar bundle. Its introduction was mainly driven by two factors:
The former will be automatically addressed by snapshots because most of the fs calls will now happen during
script/build
, whereas the latter was solved by upgrading to npm 3 back in #11897.Another caveat is that all the code that is snapshotted won't be inspectable in the DevTools, although stack traces should still be present and provide clear information about the call site where the error was thrown from.
Next steps
Overall, we are really excited about snapshots and we think we can further improve startup time by performing even more work during
script/build
. This could include eagerly constructing anAtomEnvironment
, parsing grammars and snippets, loading and parsing less style sheets, etc.Also, before merging this there are a couple of additional tasks that need to be addressed:
shouldExcludeModule
and remove unnecessarily excluded modules./cc: @atom/maintainers