Increase default transitive step to 5

[Friendseeker]

Issue

Transitive step is a mechanism intended to reduce overall zinc compile time by mass invalidating files once zinc enters too many cycles, under the assumption that when zinc enters many cycles, it would be faster to just mass invalidate instead of keeping the cycles going. We currently set transitive steps to 3 so zinc would mass invalidate after 3 cycles.

However evidences suggest that 3 is too small.

• https://github.com/JetBrains/intellij-scala/blob/9b5e23971d4d42ca6cecb59acfb66640507c2b3f/scala/compile-server/src/org/jetbrains/jps/incremental/scala/local/SbtCompiler.scala#L49
• A lot of code compiled on a small change (local variable name change) #1396 (comment)

Solution

Similar to Intellij Scala Plugin, we raise the number of transitive steps to 5.

[eed3si9n]

I'm not 100% sure. I think the original heuristics/assumption is that if you had to hop 3 times (as in one invalidation causing more, and not converging on it after 3 cycles), likely the subproject will not converge, and thus it's actually faster to compile all sources in the subproject than to repeatedly compile 10 files at a time for 5 times etc.

There are certainly cases where increasing the hops might help, but #1396 having 500+ sources in one subproject might be more of an outlier than the norm, and if it failed to converge after 5 cycles, and eventually cause full invalidation anyway, the overall performance will degrade. It might be good to sample some open source projects and show that it would improve the total performance in most projects.

[Friendseeker]

True. Maybe we should instead show a log.info() or log.warn() when transitive invalidation is triggered? Say something like "Transitive invalidation is triggered. If this behaviour is unwanted, increase the transitive steps."

But then the log might be too spammy...

[OndrejSpanel]

and if it failed to converge after 5 cycles, and eventually cause full invalidation anyway

I am not sure I understand the conditional in the sentence above, but it did not fail after 5 cycles. That said, there is an underlying issue which I am convinced is causing wrong dependencies detected, which may prevent convergence. And for sake of completeness, even in my project now, with the issue not fixed, it is enough to use _.withTransitiveStep(4) to prevent the 500 files being compiled, only 15-50 are compiled in the last step, with the total number of compiled files well below 100.

That said, I observe the total compilation time is reduced significantly on dual-core GitHub runner, where it went from 4 minutes to 1:30 in such situation, but when doing the same on my local system, which is 8-core i7-12700 with M2 SSD, the difference is much lower, 34 s with the setting and 50 without it.

show a log.info() or log.warn() when transitive invalidation is triggered

I would welcome this. I started to investigate the issue because I noticed my GitHub runner times are higher than they used to be, and I noticed that only because of receiving warnings about action minutes spent. If there would be such warning, I would know something is unusual much earlier.

I am also not sure how does #1397 interact with this.

[Friendseeker]

I am not sure I understand the conditional in the sentence above, but it did not fail after 5 cycles. That said, there is an underlying issue which I am convinced is causing wrong dependencies detected, which may prevent convergence. And for sake of completeness, even in my project now, with the issue not fixed, it is enough to use _.withTransitiveStep(4) to prevent the 500 files being compiled, only 15-50 are compiled in the last step, with the total number of compiled files well below 100.

Yes. These two PRs I made are separate from the underlying issue. Just want to reiterate that I really appreciate your hard work in #1396 (comment) and I will use that reproduction to aid our investigation.

I would welcome this. I started to investigate the issue because I noticed my GitHub runner times are higher than they used to be, and I noticed that only because of receiving warnings about action minutes spent. If there would be such warning, I would know something is unusual much earlier.

Thanks for your feedback! Still need to wait for Eugene's opinion but I believe placing a log in the last cycle is definitely something we can do.

I am also not sure how does #1397 interact with this.

#1397 is an optimization that improve's Zinc's cycle stopping condition. In your issue, I noticed that when your compilation ran out of 3 transitive steps, your compilation was already done, but Zinc was being overly pessimistic and started cycle 4.

I am currently thinking about & testing #1397 as that particular change involves Zinc's core invalidation logic. In the past, changes to that particular logic caused serious regressions such as #911 and I want to make sure that #1397 is safe.

[OndrejSpanel]

I noticed that when your compilation ran out of 3 transitive steps, your compilation was already done,

Does that mean with #1397 in place, even default _.withTransitiveStep(3) would be fine and there is no need to increase it for my project, even assuming fixing the supposed underlying issue will not reduce number of transitive steps needed?

[Friendseeker]

I noticed that when your compilation ran out of 3 transitive steps, your compilation was already done,

Does that mean with #1397 in place, even default _.withTransitiveStep(3) would be fine and there is no need to increase it for my project, even assuming fixing the supposed underlying issue will not reduce number of transitive steps needed?

That's the intention.