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

Lower browser process startup time #3453

Merged
merged 18 commits into from Sep 8, 2014

Conversation

Projects
None yet
5 participants
@kevinsawicki
Copy link
Member

kevinsawicki commented Sep 4, 2014

Taking a look at the browser process startup time.

This is the time between when atom-shell require's Atom's main file and the first window is created.

This includes the time taken to parse the command line arguments, create the native windows, setup the crash reporter, setup the system menus, and start the auto updater.

This does not include the time take to reopen editors, show the tree view, or anything that happens inside the opened window, just everything between app launch and when the window is created.

I added a new benchmark that starts the app 10 times and averages the app startup time.

Before

Startup Runs: 10
First run time: 180ms
Max time: 196ms
Min time: 173ms
Average time: 181ms

After

Startup Runs: 10
First run time: 138ms
Max time: 138ms
Min time: 117ms
Average time: 121ms

This is on a Macbook Pro on OS X 10.8.5 with 2.6 Ghz Intel Core i7 processors and 16 GB of memory.

Note that doing a new BrowserWindow() which creates the native window takes roughly 40ms on my machine so that would be the absolute minimum that we would get these times down to.

Running the benchmark

I'd be curious what results other people see so if you want to post your before and after results do the following:

On Mac OS X

  • Pull down this branch
  • Do a git checkout d7106a6b4 to checkout the commit before the changes were made
  • Run script/build
  • Run benchmark/browser-process-startup.coffee to see the before results
  • Do a git checkout ks-lower-browser-process-startup-time to checkout the latest commit
  • Run script/build again
  • Run benchmark/browser-process-startup.coffee to see the after results

On Linux/Windows

  • Pull down this branch
  • Do a git checkout fbeb9ec to checkout the commit before the changes were made
  • Run script/build
  • Run atom -f . and look for an output line in the terminal that starts with App load time:
  • Quit Atom
  • Do a git checkout ks-lower-browser-process-startup-time to checkout the latest commit
  • Run script/build again
  • Run atom -f . again and look for an output line in the terminal that starts with App load time:

The two times will be the before and after results.

What changed

Much of the time spent on my machine was doing requires of lots of files that weren't used until later on or possible never at all, but the requires were at the top of the files and so the first window wasn't being shown until all these requires were complete.

Many of these were moved to be required right before use in response to an event or request from a render process.

@kevinsawicki kevinsawicki force-pushed the ks-lower-browser-process-startup-time branch from 7fccf3e to e52a576 Sep 4, 2014

@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 4, 2014

On Windows 8.1 VM with 2 cores and 8 GB of memory, the time dropped from 230ms to 125ms.

@batjko

This comment has been minimized.

Copy link
Contributor

batjko commented Sep 4, 2014

Running the benchmark on Windows as well now, but I noticed it's important to run script/clean before each of the two builds, or there's a bunch of leftover stuff that I'm guess may interfere.

@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 4, 2014

but I noticed it's important to run script/clean before each of the two builds

You shouldn't have to, this only involves recompiling the coffeescript file, which script/build should handle.

@batjko

This comment has been minimized.

Copy link
Contributor

batjko commented Sep 4, 2014

You shouldn't have to

Hm ok, not important then. I did get a wildly different time before, but that actually could have been due to some other process' influence.

@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 4, 2014

@batjko Yeah on Windows, the very first launch seems to take longer on a fresh install.

So try launching it once and quitting, and then run a few more time afterwards to get a ballpark time.

@kevinsawicki kevinsawicki force-pushed the ks-lower-browser-process-startup-time branch from e52a576 to 2c461cc Sep 4, 2014

@batjko

This comment has been minimized.

Copy link
Contributor

batjko commented Sep 4, 2014

Ok, so I ran this on a Win8, AMD quad-core, 3.60GHz, 8GB machine and got from an average of

252ms down to 159ms.

That's a pretty respectable improvement of what... -36%?

Couple points:

  • The benchmark coffee file did not start the app 10 times, so I started it manually 10 times.
  • As noticed, this ignored the very first run, which is slower by an order of roughly 3:1.
  • The window load time (as opposed to the app load time) has not decreased (~ 2072ms). I'm guessing that's expected, as you probably only looked at app load time for now?
@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 4, 2014

The benchmark coffee file did not start the app 10 times, so I started it manually 10 times.

Yeah, I updated the issue body to mention that script is mac only for now, sorry for the confusion.

@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 4, 2014

The window load time (as opposed to the app load time) has not decreased (~ 2072ms). I'm guessing that's expected, as you probably only looked at app load time for now?

Yup window load time is next to investigate and improve.

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Sep 5, 2014

Looks like it was rebased. I checked out 7ab3e42 instead of fbeb9ec. Working on running through the benchmarks now.

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Sep 5, 2014

I ran on a 3.1GHz quad-core Core i5 iMac with 16GB of RAM.

Before

Startup Runs: 10
Max time: 366ms
Min time: 143ms
Average time: 180ms

After

Startup Runs: 10
First run time: 177ms
Max time: 177ms
Min time: 146ms
Average time: 153ms

It definitely brought down the top end significantly.

@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 5, 2014

Looks like it was rebased

Whoops, sorry about that, will update the instructions with the new commit.

@m1ga

This comment has been minimized.

Copy link

m1ga commented Sep 5, 2014

Hm, on linux the output is only:

before:
App load time: 350ms
Window load time: 1930ms

after:
App load time: 275ms
Window load time: 2070ms

@kevinsawicki kevinsawicki force-pushed the ks-lower-browser-process-startup-time branch from bade2e1 to 42f3605 Sep 8, 2014

kevinsawicki added a commit that referenced this pull request Sep 8, 2014

@kevinsawicki kevinsawicki merged commit 4bda13e into master Sep 8, 2014

1 check passed

atom Build #1671680 succeeded in 554s
Details
@kevinsawicki

This comment has been minimized.

Copy link
Member Author

kevinsawicki commented Sep 8, 2014

This will be included in the next Atom release, 0.126

@kevinsawicki kevinsawicki deleted the ks-lower-browser-process-startup-time branch Sep 8, 2014

@izuzak

This comment has been minimized.

Copy link
Member

izuzak commented Sep 8, 2014

🚄 ⚡️

@lee-dohm

This comment has been minimized.

Copy link
Member

lee-dohm commented Sep 8, 2014

💖

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