arunkumar9t2 / scabbard

🗡 A tool to visualize Dagger 2 dependency graphs
https://arunkumar9t2.github.io/scabbard
Apache License 2.0
851 stars 34 forks source link

Support incremental annotation processing #14

Open ZacSweers opened 4 years ago

ZacSweers commented 4 years ago

I remember seeing something somewhere that scabbard disables it (though didn't see anything obvious in code about this). Would be good to have the reasons documented and filing an issue for tracking it. Feel free to close if it's a technical limitation, but would be good to add this in the FAQ

arunkumar9t2 commented 4 years ago

Sorry actually wanted to provide more context around this as an issue but that is something still in my to-do.

Anyways I reran some tests and it actually appears to be incremental and I don't see any warning from kapt that incremental is disabled. My initial reason for assuming it is non incremental was because I did see incremental disabled logs from kapt when I was using an earlier version of dagger - forgot to verify after I upgraded dagger.

I did mention it to be non incremental in FAQ. Would like to do more tests before assuming it is incremental.

ZacSweers commented 4 years ago

Assuming it's just hooking into Dagger's SPI, it should be running whenever dagger runs (aka incrementally)

arunkumar9t2 commented 4 years ago

That's my understanding too :) I think it is safe to call it incremental and update the docs.

trevjonez commented 3 years ago

I believe this issue should be reopened.

Builds with scabbard produce KAPT warnings about the dot files not having originating elements.

[WARN] Issue detected with dagger.internal.codegen.ComponentProcessor. Expected 1 originating source file when generating RedactedPathToModule.dot, but detected 0: [].

after benchmarking with the gradle profiler feeding into gradle enterprise it is pretty easy to see that kapt is doing far more work when scabbard is enabled vs disabled.

configuration is roughtly:

plugins {
    id("scabbard.gradle") version "0.4.0"
}

scabbard {
    enabled = project.hasProperty("enableScabbard")
    failOnError = false
    outputFormat = "svg"
}
assembleWithScabbardAbi {
    tasks = ["app:assembleDebug"]
    gradle-args = ["-Dscan.tag.ScabbardBenchmark", "-PenableScabbard"]
    run-using = tooling-api
    daemon = warm
    warm-ups = 2
    iterations = 2
    apply-non-abi-change-to = "app/src/main/java/com/.../SomeFile.kt"
}

assembleWithoutScabbardAbi {
    tasks = ["app:assembleDebug"]
    gradle-args = ["-Dscan.tag.ScabbardBenchmark"]
    run-using = tooling-api
    daemon = warm
    warm-ups = 2
    iterations = 2
    apply-non-abi-change-to = "app/src/main/java/com/.../SomeFile.kt"
}

assembleWithScabbardNonAbi {
    tasks = ["app:assembleDebug"]
    gradle-args = ["-Dscan.tag.ScabbardBenchmark", "-PenableScabbard"]
    run-using = tooling-api
    daemon = warm
    warm-ups = 2
    iterations = 2
    apply-non-abi-change-to = "app/src/main/java/com/.../SomeFile.kt"
}

assembleWithoutScabbardNonAbi {
    tasks = ["app:assembleDebug"]
    gradle-args = ["-Dscan.tag.ScabbardBenchmark"]
    run-using = tooling-api
    daemon = warm
    warm-ups = 2
    iterations = 2
    apply-non-abi-change-to = "app/src/main/java/com/.../SomeFile.kt"
}

produced this shape in GE Build times from left to right

withScabbardAbi - 2:15, 1:17, 1:17, 1:16 withScabbardNonAbi - 1:42, 0:12, 0:11, 0:11 // kapt is up-to-date on non abi as expected so we can ignore withoutScabbardAbi - 1:27, 0:28, 0:27, 0:27 withoutScabbardNonAbi - 0:51, 0:12, 0:11, 0:11 // kapt is up-to-date on non abi as expected so we can ignore image

Pardoning the small sample size, I think it is still fairly clear that a clean and an incremental build is hurting. if we take the last run from withScabbardAbi and withoutScabbardAbi and look at the kapt task times against the clean build times.

withScabbardAbi clean build 1:04 withScabbardAbi incremental build 1:04

withoutScabbardAbi clean build 0:18 withoutScabbardAbi incremental build 0:15

so overall scabbard puts a big hit on build time but also breaks kapt incremental based on this data at least.

arunkumar9t2 commented 3 years ago

Hi @trevjonez, thank you for detailed analysis, I am reopening this and will work on this on priority for next release.

It is true that scabbard does more work when enabled due to graph generation however I understand incremental build needs to be looked at priority.