Faster zinc #206

romanowski · 2016-12-15T21:43:14Z

Changes in this PR makes zinc-based compilation comparable intellij-based incremental compilation.

For single project overhead in compilation (measured as sbt-related phases time in scalc) was reduced from over 10-15% (with bug reported by @wpopielarski even above 30%) to around 5%.

We also reduce significantly overhead in analyzing java compilation (Intellij it is still faster since it uses faster java compiler).

Overall with some changes to build infra with this changes I get similar performance that Intellij can offer for full build.

More detailed information are placed in commit messages.

Changes are done only to 2.11 sources but can be backported later to 2.10 ones.

For certain reasons this time I have to paste the below disclaimer:
THIS PROGRAM IS SUBJECT TO THE TERMS OF THE BSD 3-CLAUSE LICENSE.

THE FOLLOWING DISCLAIMER APPLIES TO ALL SOFTWARE CODE AND OTHER MATERIALS CONTRIBUTED IN CONNECTION WITH THIS SOFTWARE:
THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS “AS IS” AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,
OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR
ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.
ONLY THE SOFTWARE CODE AND OTHER MATERIALS CONTRIBUTED IN CONNECTION WITH THIS SOFTWARE, IF ANY, THAT ARE ATTACHED TO (OR OTHERWISE ACCOMPANY) THIS SUBMISSION (AND ORDINARY
COURSE CONTRIBUTIONS OF FUTURES PATCHES THERETO) ARE TO BE CONSIDERED A CONTRIBUTION. NO OTHER SOFTWARE CODE OR MATERIALS ARE A CONTRIBUTION.

romanowski · 2016-12-15T21:45:34Z

internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala

@@ -36,6 +38,26 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
  private class DependencyPhase(prev: Phase) extends GlobalPhase(prev) {
    override def description = "Extracts dependency information"
    def name = Dependency.name
+
+    val executor = Executors.newFixedThreadPool(1)


This is the simplest possible parallelization - I am open to any more sophisticated (or nicer) ones.
The only problem is that we are in the middle of scala compiler and we've got reduced configuration possibilities.

smarter · 2016-12-15T21:59:26Z

internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala

@@ -83,12 +105,11 @@ final class Dependency(val global: CallbackGlobal) extends LocateClassFile with
        def processDependency(context: DependencyContext)(dep: ClassDependency): Unit = {
          val fromClassName = className(dep.from)
          def binaryDependency(file: File, onBinaryClassName: String) =
-            callback.binaryDependency(file, onBinaryClassName, fromClassName, sourceFile, context)
+            withCallback(_.binaryDependency(file, onBinaryClassName, fromClassName, sourceFile, context))


I would much rather see the thread-handling done inside sbt itself, so scalac synchronously calls binaryDependency but the implementation of binaryDependency itself will deal with threads

It is not that simple since AnalysisCallback is managed by zinc internally.
@eed3si9n What in your opinion will best way to configure thread-handling there?

smarter · 2016-12-15T22:00:26Z

For single project overhead in compilation (measured as sbt-related phases time in scalc) was reduced from over 10-15% (with bug reported by @wpopielarski even above 30%) to around 5%.

Do you have a project somewhere that we can use as a benchmark to test this? Otherwise it's pretty hard to understand the impact of these changes.

romanowski · 2016-12-15T22:25:14Z

@smarter I wish but I cannot share one I tested this on. If you know one I will be more then happy to test.

smarter · 2016-12-15T22:26:57Z

What about compiling http://github.com/scala/scala ?

romanowski · 2016-12-15T22:34:54Z

Scala still uses sbt 0.13.13 https://github.com/scala/scala/blob/2.12.x/project/build.properties

eed3si9n · 2016-12-16T04:55:00Z

Overall, are these tune-ups focused on making IntelliJ Scala plugin's usage faster?
If so how are the compilation invoked? Are the projects built one by one, or do they run in parallel?
I want to understand some of the tradeoff we are making. For example, if we're caching things, do they increase the memory footprint? If so by how much?

eed3si9n · 2016-12-16T04:58:31Z

As per benchmarking, @cunei was working on building sbt 1 beta from source, and building projects with it. We should definitely try to test how this patch behaves with sbt's task parallelism.

romanowski · 2016-12-16T08:59:43Z

I wasn't clear. I am comparing full compilation using zinc (we've got our own integration) with full compilation in Intellij (using their incremental compiler).

Overhead (mentioned %) was measured as 100% * (time_spent_in_sbt_phases) / (whole time in scalac)

romanowski · 2017-01-19T15:26:19Z

Finally I've got some numbers.

version	API Phase	Dependency Phase	Compilation Time
1.0	1.118	0.44	11.65
this PR	0.742	0.145	11.467

Tested on https://github.com/romanowski/shapeless/tree/profiling/problem

Times exctracted using: romanowski@afd5c04

This speeds up analysing java files.

- remove multiple collection repacking and switch to mutable (faster) ones - Postpone decoding of symbol name to the end (it is called only once for each name) - create cache that keeps enclosing class, names used in that class and already processed symbols in for that class

- use associated file from symbol instead of querying for one - cache current source file with all attributes - use faster extracting symbols from type (no collections is created) - invoke callback asynchronously (in executor with one thread).

romanowski · 2017-01-25T20:37:53Z

Rebased and async calls removed. Please let me know if anything more is required.

smarter · 2017-01-25T20:45:03Z

internal/compiler-interface/src/main/java/xsbti/AnalysisCallback.java

@@ -47,4 +47,10 @@
     *       Do not depend on it, please.
     */
    boolean nameHashing();
+
+    /** Called at the end of dependency phase. Can be used e.g. to wait on asynchronous tasks. */


Do you have a concrete usecase for these callbacks?

Yes. I want to implement async calls in my client (since IIRC classDependency and binaryDependency introduce some overhead but I was able to run in parallel to analyzing compilation units).

I fear I will nee to remove it completely or introduce another callback (since I don't see where I can override it in client). @eed3si9n Any pointers?

Is your client open source?

eed3si9n · 2017-01-25T20:56:32Z

internal/zinc-classfile/src/main/scala/sbt/internal/inc/classfile/Analyze.scala

@@ -27,13 +27,15 @@ private[sbt] object Analyze {
      try { Some(Class.forName(tpe, false, loader)) }
      catch { case e: Throwable => errMsg.foreach(msg => log.warn(msg + " : " + e.toString)); None }

-    val productToClassName = new mutable.HashMap[File, String]
+    val productToClassName = mutable.Set.empty[String]


s/productToClassName/classNames/?

eed3si9n · 2017-01-25T21:01:27Z

internal/compiler-bridge/src/main/scala/xsbt/Dependency.scala

@@ -4,7 +4,9 @@
 package xsbt

 import java.io.File
+import java.util.concurrent.{ TimeUnit, Executors }


We no longer need this import correct?

eed3si9n · 2017-01-25T21:03:27Z

Thanks for benchmarking using OSS project. I'm also happy to see threading removed.

…back). Remove async calls in Dependecy phase.

romanowski · 2017-01-26T23:09:29Z

Small change - now I filter out NoSymbol in some situations.

This commit introduces changes to Scala 2.10 sources from the following PRs: * sbt#225 * sbt#216 * sbt#206 * sbt#221 It also removes a stub for 2.8 compatibility in `DelegatingReporter`. Support for Scala 2.8 compatibility is not already maintained it.

This commit introduces changes to Scala 2.10 sources from the following PRs: * sbt/zinc#225 * sbt/zinc#216 * sbt/zinc#206 * sbt/zinc#221 It also removes a stub for 2.8 compatibility in `DelegatingReporter`. Support for Scala 2.8 compatibility is not already maintained it.

This commit introduces changes to Scala 2.10 sources from the following PRs: * sbt/zinc#225 * sbt/zinc#216 * sbt/zinc#206 * sbt/zinc#221 It also removes a stub for 2.8 compatibility in `DelegatingReporter`. Support for Scala 2.8 compatibility is not already maintained it. Rewritten from sbt/zinc@5d46c1b

eed3si9n added the waiting for review label Dec 15, 2016

romanowski commented Dec 15, 2016

View reviewed changes

smarter reviewed Dec 15, 2016

View reviewed changes

romanowski added 3 commits January 23, 2017 21:25

Cache classfiles for classes.

6aca7cc

This speeds up analysing java files.

Speedup Dependency phase.

55a5d38

- use associated file from symbol instead of querying for one - cache current source file with all attributes - use faster extracting symbols from type (no collections is created) - invoke callback asynchronously (in executor with one thread).

romanowski force-pushed the faster-zinc branch 2 times, most recently from 63ff3b1 to 9082589 Compare January 25, 2017 20:35

smarter reviewed Jan 25, 2017

View reviewed changes

eed3si9n reviewed Jan 25, 2017

View reviewed changes

eed3si9n requested changes Jan 25, 2017

View reviewed changes

Add callbacks at the end of Dependecy and API phases (in AnalysisCall…

1edf1f3

…back). Remove async calls in Dependecy phase.

romanowski force-pushed the faster-zinc branch from 9082589 to 1edf1f3 Compare January 25, 2017 22:49

eed3si9n approved these changes Jan 27, 2017

View reviewed changes

eed3si9n merged commit 7215eae into sbt:1.0 Jan 27, 2017

eed3si9n removed the waiting for review label Jan 27, 2017

jvican mentioned this pull request Feb 9, 2017

Add JMH benchmarks and improve performance of Zinc phases #225

Merged

jvican mentioned this pull request Feb 21, 2017

Backport changes from several previous PRs #232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Faster zinc #206

Faster zinc #206

romanowski commented Dec 15, 2016 •

edited

Loading

romanowski Dec 15, 2016

smarter Dec 15, 2016

romanowski Dec 15, 2016 •

edited

Loading

smarter commented Dec 15, 2016

romanowski commented Dec 15, 2016

smarter commented Dec 15, 2016

romanowski commented Dec 15, 2016

eed3si9n commented Dec 16, 2016

eed3si9n commented Dec 16, 2016

romanowski commented Dec 16, 2016

romanowski commented Jan 19, 2017 •

edited

Loading

romanowski commented Jan 25, 2017

smarter Jan 25, 2017

romanowski Jan 25, 2017

smarter Jan 25, 2017

romanowski Jan 26, 2017

eed3si9n Jan 25, 2017

eed3si9n Jan 25, 2017

eed3si9n commented Jan 25, 2017

romanowski commented Jan 26, 2017

Faster zinc #206

Faster zinc #206

Conversation

romanowski commented Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romanowski Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

smarter commented Dec 15, 2016

romanowski commented Dec 15, 2016

smarter commented Dec 15, 2016

romanowski commented Dec 15, 2016

eed3si9n commented Dec 16, 2016

eed3si9n commented Dec 16, 2016

romanowski commented Dec 16, 2016

romanowski commented Jan 19, 2017 • edited Loading

romanowski commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eed3si9n commented Jan 25, 2017

romanowski commented Jan 26, 2017

romanowski commented Dec 15, 2016 •

edited

Loading

romanowski Dec 15, 2016 •

edited

Loading

romanowski commented Jan 19, 2017 •

edited

Loading