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

Overcompilation with incremental, mixed Java/Scala file edits in 1.9.0-RC1 #1311

Closed
LaCuneta opened this issue May 3, 2023 · 6 comments · Fixed by #1312
Closed

Overcompilation with incremental, mixed Java/Scala file edits in 1.9.0-RC1 #1311

LaCuneta opened this issue May 3, 2023 · 6 comments · Fixed by #1312
Assignees
Labels

Comments

@LaCuneta
Copy link

LaCuneta commented May 3, 2023

This is a follow up to this issue and the fix for the related zinc issue. I took jdsalchow's minimum repro repo, I set the sbt version to 1.3.13 (known working version from our affected project), and things worked as normal:

sbt:compile_loop> compile
[info] Compiling 3 Scala sources and 5 Java sources to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] Non-compiled module 'compiler-bridge_2.13' for Scala 2.13.6. Compiling...
[info]   Compilation completed in 8.224s.
[success] Total time: 12 s, completed May 3, 2023, 2:36:45 PM

I edited A.scala and ABase.java (just whitespace changes, no real code edits) and saw:

sbt:compile_loop> compile
[info] Compiling 1 Scala source to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] Compiling 1 Scala source and 1 Java source to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[success] Total time: 1 s, completed May 3, 2023, 2:37:04 PM

Which seems fine.

Then I updated the sbt version to 1.9.0-RC1. I cleaned the repo and the first compile was the same as before:

sbt:compile_loop> compile
[info] compiling 3 Scala sources and 5 Java sources to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] Non-compiled module 'compiler-bridge_2.13' for Scala 2.13.6. Compiling...
[info]   Compilation completed in 9.728s.
[success] Total time: 12 s, completed May 3, 2023, 2:38:16 PM

But after the non-code whitespace edits I saw:

sbt:compile_loop> compile
[info] compiling 1 Scala source and 1 Java source to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] compiling 1 Scala source and 1 Java source to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] compiling 1 Scala source and 1 Java source to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[info] compiling 3 Scala sources and 5 Java sources to /CCL-Repos/compile_loop/target/scala-2.13/classes ...
[success] Total time: 1 s, completed May 3, 2023, 2:38:30 PM

It's now invalidating all Scala and Java files and recompiling the whole source tree. For this small repo that only takes a few seconds, but for our repo it takes 90 seconds or more. Since things were working as expected in 1.3.13 this seems like a regression.

Editing one Scala file and one Java file seems necessary to trigger the issue. If I modify just one type or the other it compiles just the changed files.

@LaCuneta LaCuneta added the bug label May 3, 2023
@SethTisue SethTisue changed the title Ocercompilation with incremental, mixed Java/Scala file edits in 1.9.0-RC1 Overcompilation with incremental, mixed Java/Scala file edits in 1.9.0-RC1 May 3, 2023
@Friendseeker
Copy link
Member

Friendseeker commented Dec 19, 2023

@SethTisue Would you mind to move this issue to Zinc repo?

By the way, I am unfamiliar with how Github works, but would it be possible to grant me the privilege to close / move issues around? So I don't have to ping you over and over again for these chores.


In the meantime, for any zinc contributors reading this, let's do some brainstorming. First off, what is Carston's infinite compile fix doing? Judging by the PR description "when we have transitive invalidations include recompiled classes in the next incremental compile round.", and the code snippet below, what it is doing seems to be keep continuing until everything is recompiled when transitive invalidation is triggered.

val nextInvalidations =
if (isFullCompilation) Set.empty[String]
else
invalidateAfterInternalCompilation(
analysis,
newApiChanges,
recompiledClasses,
cycleNum >= options.transitiveStep,
IncrementalCommon.comesFromScalaSource(previous.relations, Some(analysis.relations)) _
)
// No matter what shouldDoIncrementalCompilation returns, we are not in fact going to
// continue if there are no invalidations.
val continue = nextInvalidations.nonEmpty &&
lookup.shouldDoIncrementalCompilation(nextInvalidations, analysis)

Assuming my understanding is correct, then I guess Carston's fix did not actually fix the root cause of the issue (but rather it is a last ditch effort safeguard mechanism), and there's still a need to identify the underlying root cause for infinite compilation.

@eed3si9n eed3si9n transferred this issue from sbt/sbt Dec 19, 2023
@eed3si9n
Copy link
Member

@Friendseeker I just sent you an invite to https://github.com/orgs/sbt/teams/sbt-triage-team.

@Friendseeker Friendseeker self-assigned this Dec 19, 2023
@Friendseeker
Copy link
Member

Friendseeker commented Dec 21, 2023

@LaCuneta Would you mind to provide instruction to reproduce the issue on https://github.com/NetLogo/NetLogo? I would like to do some integration testing.

Was still playing around with the previous infinite compile fix, and apparently my initial understanding of "what it is doing seems to be keep continuing until everything is recompiled when transitive invalidation is triggered." is wrong. There was a stopping condition that I overlooked.

@LaCuneta
Copy link
Author

@Friendseeker Thanks for working on this! Here are the steps to reproduce with NetLogo. NetLogo has some funky system dependencies for the full build, but I don't think they're necessary just to get compilation to happen:

git clone https://github.com/NetLogo/NetLogo.git
cd NetLogo
sbt
sbt:root> netlogo/compile
[info] creating: ~/NetLogo/netlogo-gui/target/src_managed/main/org/nlogo/window/Events.java
[info] creating: ~/NetLogo/netlogo-gui/target/src_managed/main/org/nlogo/app/common/Events.java
[info] creating ~/NetLogo/netlogo-gui/target/src_managed/main/org/nlogo/agent/ImportLexer.java
[info] compiling 137 Scala sources and 1 Java source to ~/NetLogo/parser-jvm/target/classes ...
[info] compiling 1188 Scala sources and 412 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[warn] Could not determine source for class org.nlogo.render.Renderer
[success] Total time: 106 s (01:46), completed Dec 21, 2023, 9:04:08 AM

Next edit netlogo-core/src/main/api/Workspace.scala and add def foo(x: Int): Double to the trait on line 12. Then edit netlogo-gui/src/main/workspace/AbstractWorkspace.java and add public double foo(int x) { return 10 + x; } to the abstract class on line 40.

sbt:root> netlogo/compile
[info] compiling 1 Scala source and 1 Java source to ~/NetLogo/netlogo-gui/target/classes ...
[info] compiling 83 Scala sources and 42 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[info] compiling 395 Scala sources and 209 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[info] compiling 515 Scala sources and 273 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[success] Total time: 32 s, completed Dec 21, 2023, 10:01:48 AM

And for reference, here is the same change when running sbt 1.3.13:

sbt:root> netlogo/compile
[info] Compiling 1 Scala source and 1 Java source to ~/NetLogo/netlogo-gui/target/classes ...
[info] Compiling 7 Scala sources and 43 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[info] Compiling 10 Scala sources and 43 Java sources to ~/NetLogo/netlogo-gui/target/classes ...
[success] Total time: 8 s, completed Dec 21, 2023, 9:49:05 AM

Let me know if you have any trouble with it.

@Friendseeker
Copy link
Member

Here's the result with & without the PR fix on my machine

Without the fix

[info] compiling 1 Scala source and 1 Java source to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 83 Scala sources and 42 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 395 Scala sources and 209 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 515 Scala sources and 273 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[success] Total time: 20 s, completed Dec 22, 2023, 9:02:13 p.m.

With the fix

[info] compiling 3 Scala sources and 2 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 7 Scala sources and 43 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 9 Scala sources and 6 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[info] compiling 96 Scala sources and 8 Java sources to /Users/jiahuitan/NetLogo/netlogo-gui/target/classes ...
[success] Total time: 11 s, completed Dec 22, 2023, 9:23:06 p.m.

@SethTisue
Copy link
Member

Marvelous to see this getting fixed! Thank you @Friendseeker !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants