Lint performance

3,722 views
Skip to first unread message

César Puerta

unread,
Dec 15, 2017, 2:48:14 PM12/15/17
to lint-dev
Hey!

I am looking into options for improving the performance of our lint checks, both in CI and locally, since Lint is currently one of our costliest tasks. I'm considering several ideas:
  • Sharding the execution of the detectors so that I can split them across multiple parallel jobs in CI.
  • Declaring inputs and outputs in order to make the lint task incremental and cacheable.
  • Running detectors in parallel.
  • Running lint on changed files only (using a client similar to Android Studio's).
I'm particularly interested in the idea of running lint on changed files only, since that should allow us to optimize local checks significantly while using functionality that's available out of the box (for the most part).

All these are likely non trivial, so I wanted to check with you before investigating: do you have any particular plans for improving Lint performance in the short term? Do you have any recommendations?


Thanks,
César

    Tor Norbye

    unread,
    Dec 15, 2017, 4:13:08 PM12/15/17
    to lint-dev
    Those are all good. I've been very interested in the first and third suggestions; running lint detectors in parallel, in the Gradle integration of lint. Apparently gradle now has some sort of concurrency support, and since lint is typically CPU bound, it's sad that it's all running on a single processor; it would be great if it could run in parallel.  Splitting the work up by detectors is an easy way to divide the work load, but I suspect that it's often the initial computation the lint driver does that it's going to be costly -- e.g. parsing and UAST-converting (and type-attributing) all the source files for example -- so it's probably helpful to try to distribute that work too. I haven't looked into this at all; I just heard about it from Xav, so if you or anyone else is interested in looking into it or can tell the rest of us more about it, that would be great.

    As for declaring inputs and outputs -- yes, that's been requested a lot. I used to not consider it important because lint looks at absolutely everything in your project (all source files, and dependencies, and even the build scripts themselves), so if *anything* has changed it needs to run again. So adding this caching would really only help people who run the same command twice, without changing anything, which is not a super important scenario. BUT, that was before in 3.0 I changed the default behavior for checking dependencies. Prior to 3.0, running lint on a Gradle project would have lint check the module *AND ALL ITS DEPENDENCIES* recursively, for every module. That's technically necessary to check all types of issues -- but is usually overkill and makes builds much slower. So it's an option (defaulting to off) now. And that means that we *do* have a common scenario that could benefit from caching: you've changed one file in a particular module, but lint is running on lots of library modules that haven't changed. In that case caching would really help.

    Running lint on just changed files only should be pretty easy to implement; most of the support code is already there (used from the IDE); it's really just a matter of hooking it up in the Gradle integration (e.g. making a special target, computing the changed files and passing them to lint, and invoking lint in the mode where it marks the scope for the lint analysis only include the changed file scopes).

    -- Tor

      Stéphane NICOLAS

      unread,
      Mar 8, 2018, 10:28:38 AM3/8/18
      to lint-dev
      Hi Tor,

      this issue is becoming more and more important. And I believe not only for us.

      For our CI builds we are running lint on variety of modules that didn't change at all accross builds, branches, flavors, etc and it's useless. Or CI builds take too much time and lint is a very big contributor to them.

      Add inputs, outputs, making the lint task cacheable would really be ideal for us. I believe it should not take a lot of time to make the change.
      Do you plan to add this soon ?

      Thx in advance,
       S.

      Stéphane NICOLAS

      unread,
      Mar 8, 2018, 10:28:38 AM3/8/18
      to lint-dev
      Just to give a few numbers:

      > Task :Groupon:lintGrouponRelease
      :Groupon:lintGrouponRelease took 151447ms

      BUILD SUCCESSFUL in 3m 13s
      1281 actionable tasks: 52 executed, 1229 up-to-date

      Lint takes 2m 31s out of 3m 13s ! It could be reduced to almost 0.


      On Friday, December 15, 2017 at 1:13:08 PM UTC-8, Tor Norbye wrote:

      Tor Norbye

      unread,
      Mar 12, 2018, 10:54:30 AM3/12/18
      to lint-dev
      On Thursday, March 8, 2018 at 7:28:38 AM UTC-8, Stéphane NICOLAS wrote:
      Hi Tor,

      this issue is becoming more and more important. And I believe not only for us.

      For our CI builds we are running lint on variety of modules that didn't change at all accross builds, branches, flavors, etc and it's useless. Or CI builds take too much time and lint is a very big contributor to them.

      Add inputs, outputs, making the lint task cacheable would really be ideal for us. I believe it should not take a lot of time to make the change. 

      If you or anyone else has experience with this it would be great to get some help on this.

      It doesn't look so simple to me. For example, lint can include the source code and resources of upstream modules in its analysis (with checkDependencies on); this is the only way to get accurate unused resource detection for example. Similarly, lint has checks like NewerVersionAvailable which checks the network and searches maven repositories on the network to see if there's a newer version of a library available.

      Generally, lint will get slower and slower as we add more and more checks to it; that's unavoidable, and if I have to make a choice between adding an additional check which can catch real world bugs and making the build slower, I will add the check. Compared to developer debugging hours it saves time. This is helped by the fact that lint isn't part of a normal debug build; it only runs during release builds, typically on a server. Yes, yes, of course it's better to have it run faster and not do unnecessary work, but that's less of a priority to me than adding additional checks, fixing false positives and false negatives, etc, so if this is a top priority for others it would be great to get help implementing it.

      -- Tor

        gle...@fitbit.com

        unread,
        Mar 15, 2018, 9:08:16 AM3/15/18
        to lint-dev
        Hi Tor,

        Our lint more than quadrupled in duration when we upgraded to 3.0.  I remember seeing an issue during the alphas about lint processing the same module multiple times due to how the dependency graph was setup - is that still a thing?  Ie. the following dependency setup would cause A to be checked multiple times (-> = depends on)
        A
        B->A
        C->A
        D->B
        D->A
        D->C
        E->D

        Also, based on the current version of lint, are there any performance best practices?  

        Lastly, I'd love to help out, but I don't know where to start.  Is there a good resource to start onboarding myself for poking at this, or is it more of just grab the studio source and start hacking?

        Cheers,
        -Gregorios

        Tor Norbye

        unread,
        Mar 15, 2018, 9:39:12 PM3/15/18
        to lint-dev


        On Thursday, March 15, 2018 at 6:08:16 AM UTC-7, gle...@fitbit.com wrote:
        Hi Tor,

        Our lint more than quadrupled in duration when we upgraded to 3.0.
         
          I remember seeing an issue during the alphas about lint processing the same module multiple times due to how the dependency graph was setup - is that still a thing?  Ie. the following dependency setup would cause A to be checked multiple times (-> = depends on)
         
        A
        B->A
        C->A
        D->B
        D->A
        D->C
        E->D

        I think that was turned off by default in 3.0.1, but I'm not entirely sure. It's definitely turned off by default in 3.1.
        Try adding this:

        android {
            lintOptions {
                checkDependencies false
            }
        }

        Also, based on the current version of lint, are there any performance best practices?  

        I'm tracking a 3.1-specific performance issue here;

        If you're seeing a slowdown in release builds (not running lint explicitly) that issue might affect you.

        Note also that it's a lot faster running "lintDebug" (or "lintRelease") than running "lint": the general lint target will run lint repeatedly on *each* variant and then collate all the results together. Some lint issues may only show up in some variants (for example, if you have variant specific sources or resources, or conditional logic based on the BuildConfig), which is why this is useful, but it's often not an issue so you're just doubling++ the time lint spends analyzing.

        -- Tor

          César Puerta

          unread,
          Mar 18, 2018, 5:00:03 AM3/18/18
          to lint-dev
          Hey,

          I've been wanting to look into this for a while, but have been distracted with a number of other things. In particular, I wanted to investigate the performance regression that we saw when switching from 2.3 to 3.0. I collected metrics for the time it took to process each file in a large project in 2.3 vs 3.0 and found that 3.0 was slower across the board, with one case of a 10K line file taking 0.5s in 2.3 but ~3s in 3.0. I understand that PSI is slower than ECJ, but that's a huge difference!

          We are also having trouble with lint builds in CI, since it's become the long pole in our CI verifications and we don't have a simple way to shard or optimize (even after configuring so that it won't check dependencies). One simple stopgap solution would be to allow sharding of the execution of lint task, so that we can easily select a small subset of detectors to run, and split all detectors into multiple tasks.

          Parallel execution is interesting as well. My understanding is that the AST is immutable, in which case, and provided that access to static state is synchronized, you may be able to run multiple detectors in parallel. As you suggest, you may use the worker API (https://guides.gradle.org/using-the-worker-api/) to distribute the work to multiple workers while running in Gradle.

          I think that it should be possible to declare all input and outputs in the lint tasks: many of the inputs are already available in configuration variants that AGP builds from all dependencies, and any inputs that are specific to lint should not be hard to declare. Without looking too much into the details, this is what I think this would look like:

          * Each detector declares a set of input scopes, each of which maps to a file collection containing all relevant files (including those in transitive dependencies).
          * The lint task should collect the input dependencies of all detectors and configure the task inputs based on that.
          * Once inputs and outputs are set, the task would support up-to-date checks.
          * The task can now be declared as cacheable, which enables local and remote caches.
          * An extra optimization would be to figure out what inputs have changed and what detectors are invalidated as a result, and run only those in the relevant files only.

          Also, I think that it would be generally useful to instrument lint to collect performance metrics, since that would allow us to detect regressions quickly, and pinpoint the detectors that are causing performance issues (standard or custom).

          This is annecdotal, but we've been using Lint less frequently lately because of performance reasons. Running lint locally is out of the question for us. In CI, response times are frequently driven by lint, since Lint execution takes longer than all other tasks. And our own custom rules don't work the IDE, which makes them less useful (although I can't say this is not our bug).
          If others are running into similar issues, it may make sense to spend some time looking into performance optimizations for Lint.

          I'll spend some time looking at lint performance in our build in the next couple of months and try to learn more about the issues - I'll report back if I can find anything useful!

          César

          Tor Norbye

          unread,
          Apr 9, 2018, 8:41:03 PM4/9/18
          to lint-dev
          I've heard from several people that 3.1 got much slower for them. I'm tracking that in issue https://issuetracker.google.com/71868725.
          (Note that the vital release task also got much slower in 3.1 because it was including some new complex checks, but I've removed those in 3.1.1, released this morning, such that they are only part of an explicit lint task execution, e.g. not lintVitalRelease which is called by assembleRelease.)

          There is some expected slowdown from 3.0 to 3.1 for projects that use Kotlin since in 3.0, Kotlin files were completely ignored, and in 3.1 they're analyzed (and that's generally significantly slower than Java.)  However, people are reporting dramatically slower builds (e.g. 4 minutes to 30 minuntes). If anyone else has more details on how to reproduce it (especially if it's in some open source / github project) I'd love to get my hands on it.

          -- Tor

          César Puerta

          unread,
          Apr 11, 2018, 3:42:57 PM4/11/18
          to lint-dev
          Thanks, I'll follow that issue. We've seen the regression in Java-only projects, and it seems to apply to all Java classes across the board. I'll try to collect some more information and update the issue.

          gle...@fitbit.com

          unread,
          Apr 13, 2018, 8:57:43 PM4/13/18
          to lint-dev
          Hi Tor,

          By turning off checkDependencies, don't we cripple the UnusedResources check?  As an aside: which direction does checkDependencies look (in the case of UnusedResources)?  If I'm interested in seeing if there are resources in my library modules that are not being used in my app, should I run the UnusedResources check on the app, or the library modules?

          Something else I noticed: when I run lint on a specific variant, I noticed that the task is still resolving the files that it needs for all variants.  This can take a significant amount of time for us, as we have a fairly large flavor matrix.

          Example:

          :FitbitAndroid:lintWorldNormalProdDebug > Resolve files of :FitbitAndroid:worldFastQa2DebugAndroidTestRuntimeClasspath


          We our flavor matrix is [2x, 8x, 4x]; and then we have 5 build-types (not counting androidTest) - which results in stupidly large number of variants...  I'm working on shrinking that, but in the meantime it doesn't seem quite right that linting a specific variant would need to fully resolve the files of all the others.  Thoughts?

          We've already disabled the lintVital tasks that run when we build release builds, as we currently run a full lint pass on every pull-request, as well as periodically on our master branch.

          Thanks,
          -Gregorios

          On Thursday, March 15, 2018 at 6:39:12 PM UTC-7, Tor Norbye wrote:

          Tor Norbye

          unread,
          Apr 19, 2018, 12:03:12 PM4/19/18
          to lint-dev
          On Friday, April 13, 2018 at 5:57:43 PM UTC-7, gle...@fitbit.com wrote:
          Hi Tor,

          By turning off checkDependencies, don't we cripple the UnusedResources check?  As an aside: which direction does checkDependencies look (in the case of UnusedResources)?  If I'm interested in seeing if there are resources in my library modules that are not being used in my app, should I run the UnusedResources check on the app, or the library modules?

          Check dependencies will include dependencies in the analysis, so you should run it on the app module.

          In general, for the best performance and most accurate results, you should turn checkDependencies on, and then *only* run lint on the "leaf" modules (typically app modules). E.g. instead of just running "./gradlew lint", run "./gradlew :app:lint". That will also include warnings from the dependent libraries in the single report, which is also more readable than trying to hunt around in your project tree for the HTML reports from each independent library.

          This was the original intent with the lint integration in Gradle, but the way Gradle generally works is that each checker task runs on each library and you look at reports from each library. So lots of users would just run "lint" and then complain that it was very slow, since it keeps repeating analysis in dependent libraries -- so finally I turned that off by default. But if you change things to just analyzing the leaf projects, I think things will generally work better.
           
          ALSO, in the above example I wrote "./gradlew :app:lint". Note that that will run lint over and over on *each* variant, and then finally it cross checks all the results such that it can tell you that error 1 was found in all variants, and error 2 was found in variant V. This is particularly relevant when you for example have variant specific code or resources, so it may tell you that a particular resource is unused in a particular variant. But note that it's *much* more expensive!

          So generally, you might want to just run "./gradlew :app:lintRelease" to just check the release variant.

          Something else I noticed: when I run lint on a specific variant, I noticed that the task is still resolving the files that it needs for all variants.  This can take a significant amount of time for us, as we have a fairly large flavor matrix.

          Example:

          :FitbitAndroid:lintWorldNormalProdDebug > Resolve files of :FitbitAndroid:worldFastQa2DebugAndroidTestRuntimeClasspath


          We our flavor matrix is [2x, 8x, 4x]; and then we have 5 build-types (not counting androidTest) - which results in stupidly large number of variants...  I'm working on shrinking that, but in the meantime it doesn't seem quite right that linting a specific variant would need to fully resolve the files of all the others.  Thoughts?

          Thanks for pointing this out!!

          The reason that happens is the Unused Resource check. The unused resource analyzer is used in the IDE not just for lint but also for the "Remove Unused Resources" refactoring. And note that in the IDE, there is always just *one* active variant. What happened was that some users had resources that were referenced from a non-active variant (for example, let's say it was some resources for a feature that is only available in the "pro" flavor of the app, and the user was currently in the "free" flavor in the IDE). Then the IDE would think these resources were inactive and would delete them, which obviously users weren't happy about.

          So I made a change to the unused resource detector where it also goes in and pulls in all the source folders from the *inactive* variants, and includes those as well.

          That's what we want in the IDE. But obviously NOT what we want from Gradle, where users have fine grained control about what variant they want lint to analyze -- either a specific variant, or all variants. If they run the "lint" target, it will *already* cross reference all the variants anyway, so you'd get a warning that resource "X" is unused in variant "Y". The right fix then is to move the resource to a variant specific resource folder.

          I've fixed this now such that the add-inactive-source-folders code is *only* done when the detector us running in the IDE. That will be included in 3.2 canary 13.

          Cesar also reported on the above bug that lint analyzing test sources is accounting for 40% of his built time (after the annotation-lookup-caching was added, which I think will be released in canary 12 this week). The reason this happens is that the unused resource detector includes test sources. That's to make sure that we don't mark a resource as unused if it's still referenced from a test.

          I was really torn about how to fix this. The reason it does that is that users complained that lint would mark resources as unused if they were still referenced from tests. And I assume that means that some developers are placing resources into their production that are only referenced from tests, and that's what they want. So if I simply make the unused resource detector stop flagging them, that might make some users unhappy. On the other hand, this is a pretty big overhead for projects with lots and lots of test sources, and I suspect it's pretty uncommon for users to have resources that are intentionally only referenced from tests.

          What I ended up with is that I added a new flag, "ignoreTestSources", which you can set to tell lint to completely ignore tests. 

          Note that lint *already* has a flag related to test sources, "checkTestSources", which is similar to "checkDependencies" and "checkGeneratedSources" -- these flags tell lint to run *all* the enabled lint checks on all the test sources, the dependent library sources, and the generated sources respectively. All of these are off by default.

          But note that even if checkTestSources isn't turned on, lint will run *some* lint checks on the test sources: the lint checks that are intended for tests, e.g. a lint check that is checking tests themselves for problems (such as calling assert instead of assertEquals, since assertions may not be enabled and you might not actually be checking what you think you're checking). There's a Scope.TEST_SOURCES flag which lint uses to control whether a detector should be run on test sources.

          Right now there is exactly one check in lint which sets this flag: UnusedResourceDetector -- but I know there are some third party checks which use it too. If I were to just remove that scope from UnusedResourceDetector, it would no longer check test sources and builds would be faster -- but also would miss scenario where a test keeps a resource from being unused.

          The new flag will let you say "I really really want you to ignore tests", so the test sources would be skipped. This is really a performance tweaking flag, but for those with really long lint times who know they don't want test analysis, possibly quite useful. I haven't integrated it yet so this missed canary 13, but I'm hoping for canary 14.   

          If you have any thoughts on this, especially opinions regarding what to do about the unused resource detector and test-only references to resources, I'd love to hear it.

          -- Tor

            César Puerta

            unread,
            Apr 19, 2018, 8:46:21 PM4/19/18
            to lint-dev
            Thanks for the detailed explanation, Tor!

            Regarding resources that are used only in tests, I'm wondering whether that's a use case that justifies the default extra cost in lint - especially since it may be so large and many users may not realize they can use the flag safely to save the cost. Also, I'm not sure Lint should not flag these resources by default:
            • If it's an androidTest test, the resources can be moved to androidTest/res.
            • If a unit test using Robolectric, there are probably better options than packaging the resources in a production module: either updating AGP to parse resources under test/res, or just ignoring them explicitly (in order to acknowledge that they are only meant for use in tests).
            Would it make sense to ignore test sources by default, and instead add a "includeTestSources" flag for those folks that explicitly want to be able to bundle test resources in their app?

            César

            Tor Norbye

            unread,
            Apr 26, 2018, 10:12:30 PM4/26/18
            to lint-dev

            On Thursday, April 19, 2018 at 5:46:21 PM UTC-7, César Puerta wrote:
            Thanks for the detailed explanation, Tor!

            Regarding resources that are used only in tests, I'm wondering whether that's a use case that justifies the default extra cost in lint - especially since it may be so large and many users may not realize they can use the flag safely to save the cost. Also, I'm not sure Lint should not flag these resources by default:
            • If it's an androidTest test, the resources can be moved to androidTest/res.
            • If a unit test using Robolectric, there are probably better options than packaging the resources in a production module: either updating AGP to parse resources under test/res, or just ignoring them explicitly (in order to acknowledge that they are only meant for use in tests).
            Would it make sense to ignore test sources by default, and instead add a "includeTestSources" flag for those folks that explicitly want to be able to bundle test resources in their app?

            I went back and forth on this one.

            I started reversing the flag logic. But then I came across the several bugs people had filed when lint told them resources were unused when they were referenced from tests. If I just switch this default, those same users will get it.  Of course it would be nice if lint would instead tell them "hey, this resource is *only* used from tests; you should instead <suggestion>". But to compute that, I'd need to scan all the test sources anyway, so no performance win.  In the end, I think that that this is mainly a project in really large projects, and those kinds of developers are more likely to find out how to tune and explicitly opt in/out of features anyway. We should make sure this is covered in some of the performance tuning guides for Android Gradle builds, and maybe have one for lint specifically. 

            Anyway, the includeTestSources flag made it into canary 14, so once that goes live you should be able to opt out of tests. And canary 12 shipped earlier this week; that had a pretty important caching fix in it around annotation lookup that should help a lot.

            -- Tor

              Cristian Garcia

              unread,
              May 30, 2018, 7:45:27 PM5/30/18
              to lint-dev
              El viernes, 27 de abril de 2018, 4:12:30 (UTC+2), Tor Norbye escribió:
              > On Thursday, April 19, 2018 at 5:46:21 PM UTC-7, César Puerta wrote:
              > Thanks for the detailed explanation, Tor!
              >
              >
              > Regarding resources that are used only in tests, I'm wondering whether that's a use case that justifies the default extra cost in lint - especially since it may be so large and many users may not realize they can use the flag safely to save the cost. Also, I'm not sure Lint should not flag these resources by default:
              > If it's an androidTest test, the resources can be moved to androidTest/res.If a unit test using Robolectric, there are probably better options than packaging the resources in a production module: either updating AGP to parse resources under test/res, or just ignoring them explicitly (in order to acknowledge that they are only meant for use in tests).
              > Would it make sense to ignore test sources by default, and instead add a "includeTestSources" flag for those folks that explicitly want to be able to bundle test resources in their app?
              >
              >
              > I went back and forth on this one.
              >
              >
              > I started reversing the flag logic. But then I came across the several bugs people had filed when lint told them resources were unused when they were referenced from tests. If I just switch this default, those same users will get it.  Of course it would be nice if lint would instead tell them "hey, this resource is *only* used from tests; you should instead <suggestion>". But to compute that, I'd need to scan all the test sources anyway, so no performance win.  In the end, I think that that this is mainly a project in really large projects, and those kinds of developers are more likely to find out how to tune and explicitly opt in/out of features anyway. We should make sure this is covered in some of the performance tuning guides for Android Gradle builds, and maybe have one for lint specifically. 
              >
              >
              > Anyway, the includeTestSources flag made it into canary 14, so once that goes live you should be able to opt out of tests. And canary 12 shipped earlier this week; that had a pretty important caching fix in it around annotation lookup that should help a lot.
              >
              >
              > -- Tor

              I've been taking a look on the code and trying to figure out how I would implement lint to make it cacheable. I'll share my thoughts here even if the may be too simplistic.


              lint.dependsOn lintAnalyze

              lintAnalyze
              inputs:
              - project/module dir
              - Lint.xml Config file
              - Configured reports to be generated
              - Flags
              - ¿Baseline?
              OutputsDir:
              - the folder with the result reports, XML or an internal parcelable file is needed anyway.
              It does the actual linting.
              Using as inputsDir all the project/module dir is not optimal, but it's a way to start. Later on we could go for incremental linting.

              Lint reads the lintAnalyze result, parse it to generate the `List<Warning>` and does the print/throw stuff, so the same situation leads to the same build result even if using a cached `lintAnalyze` result.
              Lint task don't have outputs or inputs, it's not cacheable.

              Cristian Garcia

              unread,
              May 30, 2018, 7:45:27 PM5/30/18
              to lint-dev
              Hi Tor, I'm profiling android builds around my company and the biggest task on every project ( excluding UI Tests ) is Lint..

              They key is that all our apps are multi-module right now, so, I'm sorry to insist, adding the Inputs and Outputs to lint will help a lot, at least some modules would skip running lint because it's cached.

              Is it possible?


              El viernes, 27 de abril de 2018, 4:12:30 (UTC+2), Tor Norbye escribió:

              Cristian Garcia

              unread,
              Jun 6, 2018, 8:17:10 AM6/6/18
              to lint-dev
              Trying to improve my last suggestion, and after spending a couple of hours trying to understand both:
              https://android.googlesource.com/platform/tools/build/+/master/gradle/src/main/groovy/com/android/build/gradle/tasks/Lint.groovy

              The important thing to `LintCliClient` is the `LintRequest` that you are extending on the Gradle plugin, I didn't get to list all the details of the request used for Lint to analyze, but I think the Input for the (new) task `lintAnalyze` should be exactly the `LintGradleRequest`, but it's not that simple.
              Gradle Task inputs need to be one of:
               - `Serialzable`
               - `File *` ( folder or file )
               - `Iterable<File>*`
               ( files, classpath, compileClasspath )
               - `Nested`.. this could be the key, if you cannot make `
              LintGradleRequest` serializable.


              If generating the `LintGradleRequest` it's too complex we could have another task, and use the output of this first non-cacheable task as the input for `lintAnalyze`.

              In that way, without breaking backwards compatibility we would have:

              lintGenerateRequest: fast, non-cacheable
              lintAnalyze: slow but cacheable, not yet incremental, is the one that runs `LintCliClient`
              lint: depends on both, and just analyze the `
              lintAnalyze` xml report to show warnings/errors on the console, and may make the build fail.


              What I haven't found yet is the baseline, but I guess is more related to the reporter than the analyzer. Is it in anyway sent through the same `LintRequest`?

              Does it make any sense to you?

              Tor Norbye

              unread,
              Jun 6, 2018, 4:47:57 PM6/6/18
              to lint-dev
              On Wednesday, June 6, 2018 at 5:17:10 AM UTC-7, Cristian Garcia wrote:

              FYI, we don't use the branch "master"; we use the branch "studio-master-dev" (for reasons I can't remember), and in particular, it looks like master is trailing far behind our active studio-master-dev.

              The reason I spotted that is that the above file is named "Lint.groovy" and we got rid of all the groovy files in the build system a long time ago.

              And that means that there's a pretty big gap between what you've studied and what's in there now.
              In 3.1 I had to make a pretty large change to the way lint is run in Gradle. In order to support analyzing Kotlin files, I had to include the Kotlin compiler on lint's classpath, but it would interfere with the Kotlin compiler classes loaded by the Kotlin Gradle plugin. (And I couldn't jarjar this; the classes have to be in their original namespace since when lint runs in the IDE we want it to talk to the Kotlin AST from the IDE).

              That meant basically wrapping most of lint behind a custom class loader.

              The important thing to `LintCliClient` is the `LintRequest` that you are extending on the Gradle plugin, I didn't get to list all the details of the request used for Lint to analyze, but I think the Input for the (new) task `lintAnalyze` should be exactly the `LintGradleRequest`, but it's not that simple.
              Gradle Task inputs need to be one of:
               - `Serialzable`
               - `File *` ( folder or file )
               - `Iterable<File>*`
               ( files, classpath, compileClasspath )
               - `Nested`.. this could be the key, if you cannot make `
              LintGradleRequest` serializable.


              If generating the `LintGradleRequest` it's too complex we could have another task, and use the output of this first non-cacheable task as the input for `lintAnalyze`.

              In that way, without breaking backwards compatibility we would have:

              lintGenerateRequest: fast, non-cacheable
              lintAnalyze: slow but cacheable, not yet incremental, is the one that runs `LintCliClient`
              lint: depends on both, and just analyze the `
              lintAnalyze` xml report to show warnings/errors on the console, and may make the build fail.


              What I haven't found yet is the baseline, but I guess is more related to the reporter than the analyzer. Is it in anyway sent through the same `LintRequest`?

              Does it make any sense to you?

              I'm wondering if this will be enough. Lint also looks at the build.gradle files themselves, and potentially *any* files in the source tree (not just java/xml/kotlin/etc).
              It currenty doesn't compute this information up front; it's discovered during lint's recursive analysis. So it's hard to have that as a state object.

              -- Tor

              Cristian Garcia

              unread,
              Jun 7, 2018, 2:49:35 AM6/7/18
              to lint-dev
              Really?? The branch is "studio-master-dev"...in going to my crying corner...

              Thanks for noticing.

              About adding the gradle files sees easy but any other file... It seems it would make the issue harder.
              I would assume the list of files could only be computed after loading the classes on AST, isn't it?
              Could you point me to the right place to look how this: AST, exploration, run linters work?
              I want to see if that can be split in steps (at least exploration and analysis) and if it's worth, because I guess just loading the class on AST to explore it would consume most of the time, and that currently the linter doesn't need to load anything because you reuse everything from the exploration phase.

              Tor Norbye

              unread,
              Jul 5, 2018, 12:01:13 PM7/5/18
              to lint-dev
              I met with the Gradle team last week to brainstorm possible solutions here. We have several ideas, though there's no low hanging fruit. The biggest challenge is that lint likes to look at the global project graph (how else can it consider the minSdkVersion from the app module when analyzing a lib module), and that's not How Gradle Works. Some of the things we're considering is having lint parameterize more of the behavior can be modified by downstream modules. For example, when analyzing a lib module lint could *record* an error saying that it applies if minSdkVersion > 15 and targetSdkVersion < 23 and store that somewhere. When lint runs on the app module it can load the state from the lib module's single analysis and then filter those errors based on the actual observed value of those config values.  We also discussed trying to make lint behave more incrementally such that on a CI server or for a project with lots of modules where only a few things have changed, it tries not to re-run. For that we need to be able to capture better what lint's inputs and outputs are; again, that's challenging since lint looks outside the current project, but apparently we have abilities to run code to determine those things, and the Gradle team will help with this task.

              -- Tor

              Cristian Garcia

              unread,
              Jul 5, 2018, 3:45:50 PM7/5/18
              to lint-dev
              I was afraid of your silence, but it seems like really good news!
              Thanks for trying to fix it!

              In our case we ended using `./gradlew build --exclude-task lint` and running lint in another worker on the CI. It's not amazing but our builds are times faster now.
              We'll work with the Gradle enterprise trial hopefully next week, but I think lint will still be out biggest issue.

              César Puerta

              unread,
              Jul 5, 2018, 5:34:29 PM7/5/18
              to lint-dev
              That's great news, thanks. Also, it would be super helpful for CI environments if you could enable some sort of sharding mechanism to facilitate breaking down lint tasks into multiple jobs - either by file (for detectors that don't need to look at all files) or by detector.

              César
              Reply all
              Reply to author
              Forward
              0 new messages