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

Speed up start up and reload times #3115

Merged
merged 4 commits into from Apr 19, 2017

Conversation

Projects
None yet
2 participants
@jvican
Copy link
Member

commented Apr 18, 2017

This is a port of #3019 to sbt 1.0. To see the previous discussion, please have a look at that PR.

These changes are optimizations that affect the following sbt subsytems:

  1. Sbt parser.
  2. Plugin discovery.

A concise explanation for every change is provided in the commit message. In
short, this PR makes sure that we don't load the whole Scala reflect universe
to check whether a plugin has a autoImport member or to parse a sbt file.

In informal measurements, these changes affect around ~12-14% of the whole
startup time. These are the results before:

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11310 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  29.66s user 0.56s system 297% cpu 10.171 total

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11310 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  30.52s user 0.46s system 299% cpu 10.337 total

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11310 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  28.85s user 0.43s system 293% cpu 9.988 total

And after:

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[warn] Executing in batch mode.
[warn]   For better performance, hit [ENTER] to switch to interactive mode, or
[warn]   consider launching sbt without any commands, or explicitly passing 'shell'
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11355 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  24.83s user 0.51s system 286% cpu 8.831 total

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[warn] Executing in batch mode.
[warn]   For better performance, hit [ENTER] to switch to interactive mode, or
[warn]   consider launching sbt without any commands, or explicitly passing 'shell'
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11355 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  24.46s user 0.46s system 287% cpu 8.674 total

jvican in /data/rw/code/scala/scala
> $ time sbt "exit"
[warn] Executing in batch mode.
[warn]   For better performance, hit [ENTER] to switch to interactive mode, or
[warn]   consider launching sbt without any commands, or explicitly passing 'shell'
[info] Loading global plugins from /home/jvican/.sbt/0.13/plugins
[info] Loading project definition from /data/rw/code/scala/scala/project/project
[info] Loading project definition from /data/rw/code/scala/scala/project
[info] Resolving key references (11355 settings) ...
[info] *** Welcome to the sbt build definition for Scala! ***
[info] Check README.md for more information.
sbt -mem 2048 "exit"  25.12s user 0.45s system 290% cpu 8.802 total
@jvican

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2017

The error https://travis-ci.org/sbt/sbt/jobs/223197672 is reproducible from latest master locally, which hints this PR is not causing it.

jvican added some commits Mar 14, 2017

Don't use runtime universe to discover autoImport
The previous implementation was using the Scala runtime universe to
check whether a plugin had or not an `autoImport` member. This is a bad
idea for the following reasons:

* The first time you use it, you class load the whole Scalac compiler
  universe. Not efficient. Measurements say this is about a second.
* There is a small overhead of going through the reflection API.

There exists a better approach that consists in checking if `autoImport`
exists with pure Java reflection. Since the class is already class
loaded, we check for:

* A class file named after the plugin FQN that includes `autoImport$` at
  the end, which means that an object named `autoImport` exists.
* A field in the plugin class that is named `autoImport`.

This complies with the plugin sbt specification:

http://www.scala-sbt.org/1.0/docs/Plugins.html#Controlling+the+import+with+autoImport
Reuse the same global instance for parsing
The previous implementation was instantiating a toolbox to parse every
time it parsed a sbt file (and even recursively!!!).

This is inefficient and translates to instantiating a `ReflectGlobal`
every time we want to parse something.

This commit takes another approach:

1. It removes the dependency on `ReflectGlobal`.
2. It reuses the same `Global` and `Run` instances for parsing.

This is an efficient as it can get without doing a whole overhaul of it.
I think that in the future we may want to reimplement it to avoid the
recursive parsing to work around Scalac's bug.
Avoid the use of `synchronized` while parsing
Previous commit used `synchronized` to ensure that the global reporter
was not reporting errors from other parsing sessions. Theoretically,
though, sbt could invoke parsing in parallel, so it's better to ensure
we remove the `synchronized` block, which could also be preventing some
JVM optimizations.

The following commit solves the issue by introducing a reporter id.

A reporter id is a unique identifier that is mapped to a reporter. Every
parsing session gets its own identifier, which then is reused for
recursive parsing. Error reports between recursive parses cannot collide
because the reporter is cleaned in `parse`.
Add global toolbox to parse sbt files
This change was proposed by Jason in case that the new parsing mechanism
implemented later on has to be reverted. This change provides a good
baseline, but it's far from ideal with regard to readability of the
parser and performance.

@jvican jvican force-pushed the scalacenter:speed-up-startup-time-1.0 branch from cfd0436 to 2f61114 Apr 18, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2017

The error has been fixed and a more stable implementation of the autoImport detector has been added.

I think GitHub has a bug in the UI and is showing the commits in an incorrect order. The CI has kicked in for the last one.

@jvican

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2017

@eed3si9n @dwijnand This is ready for review again. Please, can you have a look?

@eed3si9n eed3si9n merged commit cc54121 into sbt:1.0.x Apr 19, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n removed the ready label Apr 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.