errorprone cause large slowdown of compileJava/compileTestJava [LUCENE-9879]

[asfimport]

Errorprone hooks itself into the java compiler (compileJava/compileTestJava). So this static analysis runs, even if you are just recompiling to run tests.

It causes javac to run 3x slower at the moment.

With errorprone:

188.26 sec.  compileTestJava
180.24 sec.  compileJava

Without errorprone:

  52.54 sec.  compileTestJava
  49.26 sec.  compileJava

The worst part is: if you look at the config, essentially all error-prone checks are disabled. So we aren't getting value out of it, and it slows builds significantly.

I'd like to have a discussion about it:

• should we keep it enabled?
• should it really hook into compileJava/compileTests or maybe be a separate task (e.g. precommit only)

I'm raising this because it is so slow, that e.g. Uwe mentioned on other issues the possibility of switching to ECJ compiler to try to speed up builds, but, it turns out javac is actually fast. The slowness is error-prone.

For sure if we are going to use it, I'd like to at least turn on checks and get value out of the thing. But it is unclear to me how many of these checks aren't already met by javac or ecj (which frankly, we still haven't enabled to do good analysis yet, although the situation is improving)

cc @uschindler @dweiss

---

Migrated from LUCENE-9879 by Robert Muir (@rmuir), resolved Mar 28 2021
Linked issues:

• Achieve build system perfection (with progress) [LUCENE-9871] #10910
• SOLR-16258

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

In an ideal world I think, we'd let javac do compileJava/compileTestJava fast and let precommit run separate ecj lint, with errorprone hooked into the ECJ compile.

But it seems this is not possible: https://errorprone.info/docs/installation#eclipse

Nevertheless, I think its wrong to slow down the compile/test/debug loop of the build so much. So IMO if this checker has value, we should turn on and enable the checks, and consider adding it as a separate pass to precommit, even if it means adding adding additional time there.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Related issue, particularly this comment: google/error-prone#994 (comment)
Seems to correlate to what we see, even with all checks disabled, it is painful.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

Oh... so that's why it's slow!

I see three options:

1. Moving ecj to a separate check (this will require javac recompilation). This will only slow down the full check cycle (but will fail sooner if you have errors).

2. Making ECJ run only if you're also running tests... This is doable from gradle side.

3. Making ECJ run only on CI runs (when -Ptests.nightly=true).

I'm in favor of option 3.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

Left a PR implementing option (3).

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Option 3 seems fine, but maybe with a separate flag. Tests.nightly is fine, but too specific.

Thanks Robert: this explains why it got so slow over the last months! Previously compileCore was really fast, especially for lucene-core. Now it takes ages. I have the feeling errorprone has some quadratic/exponential runtime on number or complexity of files.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

We can add a separate flag too, of course ("build.validation.errorprone") but I'd limit the number of these flags... There's so many of them already. A nightly run seems like a good aggregate for "everything that's expensive"?

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Yes. I think that's easiest to do without reconfigure jobs.
We only have one nightly job in ASF Jenkins. Everything else works fast.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

Thanks Robert: this explains why it got so slow over the last months! Previously compileCore was really fast, especially for lucene-core. Now it takes ages. I have the feeling errorprone has some quadratic/exponential runtime on number or complexity of files.

Each check looks to be standalone thing, and tree-based. And there are zillions of individual, very specific checks.

So its like having zillions of DOM-based checks, versus something smarter like single-pass SAX. If this is why it is slow, I think it will only get worse over time as they add more checks.

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

I committed this in as it was, @uschindler. If this ever becomes a problem let me know and we'll introduce a separate property.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

I think it's all fine. We can close this issue!

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

Closing after the 9.0.0 release