google / dagger

A fast dependency injector for Android and Java.
https://dagger.dev
Apache License 2.0
17.42k stars 2.01k forks source link

Faster Execution on CPUs with many cores #1456

Closed samzurcher closed 5 years ago

samzurcher commented 5 years ago

We use Dagger2 in a fairly large Android application written in Kotlin. We use the dagger-compiler with kapt3. The kapt step takes unfortunately a significant amount of time in our overall build. Looking at the CPU history during the build, it is obvious that only very few CPU cores are used; meaning that the build is a lot slower than it actually could be.

Without having in-depth knowledge of Dagger2, it seems that that at least parts of the dagger annotation processing could be done in parallel.

Questions:

  1. Is there a conceptual problem in increasing the parallelism for the dagger annotation processing?
  2. Would it be possible, to add support for better parallelism? E.g. using parallelStream() in the correct places?

Thanks a lot for looking into that. I'm sure I'm not the only developer who would enjoy writing more code and less waiting :-).

ronshapiro commented 5 years ago

Our cursory look at this in the past has deemed it to be much harder than ideally possible for a few reasons:

I'm open to suggestions of what could be done though. Formatting generated code is something that @netdpb has interest in parallelizing, but I don't think that's on his plate in the near term.

On Thu, Mar 28, 2019, 8:37 AM Sam Zurcher notifications@github.com wrote:

We use Dagger2 in a fairly large Android application written in Kotlin. We use the dagger-compiler with kapt3. The kapt step takes unfortunately a significant amount of time in our overall build. Looking at the CPU history during the build, it is obvious that only very few CPU cores are used; meaning that the build is a lot slower than it actually could be.

Without having in-depth knowledge of Dagger2, it seems that that at least parts of the dagger annotation processing could be done in parallel.

Questions:

  1. Is there a conceptual problem in increasing the parallelism for the dagger annotation processing?
  2. Would it be possible, to add support for better parallelism? E.g. using parallelStream() in the correct places?

Thanks a lot for looking into that. I'm sure I'm not the only developer who would enjoy writing more code and less waiting :-).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/dagger/issues/1456, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwY3dYPBGVAoU61CROCYDPjNFu9Fa4aks5vbLdwgaJpZM4cQDfq .

samzurcher commented 5 years ago

Thanks a lot for the reply and the analysis. You guys know dagger a lot better than I do (just glanced at the source today), so unfortunately no suggestions.

Do you guys have any configurable debug output that would reveal more about the timing of the sub steps executed? If there were such a debug output, I could run it on my project and see where time really gets eaten... - maybe that could be taken as an input on what really hurts - and maybe you would see an option for improvement based on that...

ronshapiro commented 5 years ago

We don't have anything to currently capture that, but we could add that. Usually hooking up a profiler is the most helpful I've found, since it gives you much richer information than just timestamps. I think Jetbrains has a profiler that works well with gradle builds but I'm not exactly sure.

On Thu, Mar 28, 2019 at 10:20 AM Sam Zurcher notifications@github.com wrote:

Thanks a lot for the reply and the analysis. You guys know dagger a lot better than I do (just glanced at the source today), so unfortunately no suggestions.

Do you guys have any configurable debug output that would reveal more about the timing of the sub steps executed? If there were such a debug output, I could run it on my project and see where time really gets eaten... - maybe that could be taken as an input on what really hurts - and maybe you would see an option for improvement based on that...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/google/dagger/issues/1456#issuecomment-477615272, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwY3ZcQuSsuhd3QeiixlqScB0qfHC-gks5vbM-zgaJpZM4cQDfq .

cgdecker commented 5 years ago

I experimented a little with trying to parallelize some stuff in the annotation processor and found that the javac APIs used in annotation processing are not just documented to be non-threadsafe (but actually relatively easy to work around), they really are extremely non-threadsafe throughout. I don't think we could parallelize without first doing something like figuring out a way to make an immutable, thread-safe copy of all the data we'll actually need from those APIs as a first step then use and then using that throughout the parallelized code. But that seems like it would be a very large project, to say the least. And even then there are other roadblocks.

ronshapiro commented 5 years ago

Given Colin's assessment, let's close this issue as infeasible. There's much optimization work to be done to make Dagger faster in a serial manner. Let's keep pushing in that direction before we attempt any parallelism.