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

Support incremental annotation processing #14

Open
ZacSweers opened this issue Dec 30, 2019 · 5 comments
Open

Support incremental annotation processing #14

ZacSweers opened this issue Dec 30, 2019 · 5 comments
Assignees
Labels
module:processor Label to mark all discussions about scabbard's annotation processor question Further information is requested

Comments

@ZacSweers
Copy link
Contributor

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
Copy link
Owner

arunkumar9t2 commented Dec 31, 2019

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.

@arunkumar9t2 arunkumar9t2 added module:processor Label to mark all discussions about scabbard's annotation processor question Further information is requested labels Dec 31, 2019
@ZacSweers
Copy link
Contributor Author

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

@arunkumar9t2
Copy link
Owner

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

@trevjonez
Copy link

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
Copy link
Owner

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.

@arunkumar9t2 arunkumar9t2 reopened this Jan 17, 2021
@arunkumar9t2 arunkumar9t2 added this to To do in Scabbard Development via automation Jan 17, 2021
@arunkumar9t2 arunkumar9t2 self-assigned this Jan 17, 2021
@arunkumar9t2 arunkumar9t2 added this to the 0.5.0 milestone Jan 17, 2021
@arunkumar9t2 arunkumar9t2 removed this from the 0.5.0 milestone Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:processor Label to mark all discussions about scabbard's annotation processor question Further information is requested
Projects
Development

No branches or pull requests

3 participants