fix: Revert "chore: update Scala 2 version"

[magnus-madsen]

Unfortunately there is a severe performance regression in the newer Scala compiler.

See e.g.:

I can confirm this locally too.

[magnus-madsen]

@michelou If you have time, it might be worth trying to track down the specific version of Scala that is broken. Running gradlew clean jar should easily reveal the problem (compilation time typically jumps from 1min to 6min) on my machine.

[michelou]

@magnus-madsen This is bad news. I'll have a look.

[magnus-madsen]

Best guesses:

• Random Scala issue.
• Something with macros.
• Something with Gradle tooling.

[michelou]

@magnus-madsen I confirm the severe performance regression in the Scala 2 compiler starting with release 2.13.6 (you had to revert my PR #4896 which is based on Scala 2.13.10).

I did some profiling (with VisualVM) during the generation of flix.jar and observed a ~100x slow-down in method unreachableCase (file scala/tools/nsc/transform/patmat/MatchAnalysis.scala) whose implementation has changed a lot between Scala 2.13.5 and Scala 2.13.6+. More precisely, the call to method eqFreePropToSolvable introduces a huge increase in execution time (I skip the details here).

I will open an issue on scala/bug and ask @lrytz to validate my diagnosis and propose a fix.

[magnus-madsen]

Thanks @michelou! That's some detailed analysis and an interesting find!

[paulbutcher]

@michelou can you point me at the Scala bug you created (I don't seem to be able to track it down)?

Thanks!

[michelou]

Here are 3 sections to resume the case :

1. Retrospective
   Flix currently depends on version 2.13.5 of the Scala library (cf. line 15 in file build.gradle) but current version of the Scala library is 2.13.10 (and version 2.13.11 is coming very soon).

   Concretely this Scala issue is directly related to the usage of option -Ypatmat-exhaust-depth (line 42 in file build.gradle) which is responsible for the increase in execution time during the generation of assembly file flix.jar.
   As a reminder option -Ypatmat-exhaust-depth governs how much time we spend analyzing matches for unreachability/exhaustivity (see source file Logic.scala).
   NB. If we consider the release based dissection we can deduce that the breaking change occurred between February and May 2021:

   • Scala 2.13.5 was released on February 22, 2021.
   • Scala 2.13.6 was released on May 17, 2021.
   • Scala 2.13.10 was released on October 13, 2022.

   Note: At that point (November 2022) I saw 3 alternative to circumvent this problem (namely the impossibility to depend on Scala 2.13.10):
   a) we do not use option -Ypatmat-exhaust-depth (an ahdoc workaround I suggest on my GitHub page CONTRIBUTIONS.md).
   b) we submit an issue to scala/bug; this corresponds to the report I initially planned to submit to Lukas.
   c) we switch to Scala 3 (middle-term decision)
   My choice was/is point b) (I use the solution under point a) when working with Scala 2.13.10).

2. Reporting
   In short I suspected the origin of the problem is located in the "patmat" phase of the nsc compiler (file PatternMatching.scala). More precisely we have

                        /---> MatchingAnalysis
        PatternMatching ----> (others)
                        \---> Solving ---> Logic
   

   NB. Changes between version 2.13.5 and 2.13.10 are relatively small. Last change dates are :

   • Logic.scala --> December 30, 2021
   • MatchAnalysis.scala --> December 21, 2022
   • Solving.scala --> April 29, 2021 (reminder: 2.13.5 -> February 2021 and 2.13.6 -> May 2021)

   My guess after (visually) analysing the source code (resp. comparing 2.13.5 and 2.13.10 versions of the sources):

   • Logic.scala --> very low probability
   • MatchAnalysis.scala --> method "exapandMdel(solution: Solution): ..." may need further analysis (otherwise nothing special)
   • Solving.scala --> very high probability (many changes)

   As reported in my comment from November 17, 2022 I could trace the issue down to line 501

   reachable = hasModel(eqFreePropToSolvable(and))
   

   of the source file MatchAnalysis.scala with the definition of eqFreePropToSolvable on line 311

   def eqFreePropToSolvable(p: Prop): Solvable = { ... }
   

   of the source file Solving.scala.

   Note: Change history of file Solving.scala is

   • adriaanm created the file Solving.scala on March 4, 2013 (see scala/scala@0a3219b9#diff-a28dc9b0)
   • dwijnand committed significant changes on March 3, 2021 with comment "Fix bad performance on complex patmat" (see scala/scala@6eaf217)
     Reminder: 2.13.5 -> February 2021 and 2.13.6 -> May 2021 and dwijnand's change was commited on March 3, 2021

3. Final word
   Finally I stopped my investigations at the beginning of December 2022 when I discovered that user "fredfp" (Arnalut Burlet) had already encountered/reported the same issue #12499 on November 23, 2021.
   Meanwhile (starting on September 2022) the issue has got more attention and user "KisaragiEffective" created PR#10258 a week ago.

   In conclusion this issue will hopefully get a solution very soon.

[SethTisue]

Fix is merged and Scala 2.13.11 should be out within a few weeks (or at worst a month or two). You might like to test with the currently Scala 2.13 nightly

[magnus-madsen]

Fix is merged and Scala 2.13.11 should be out within a few weeks (or at worst a month or two). You might like to test with the currently Scala 2.13 nightly

Thanks for the heads up- we should give it a spin :)

[michelou]

@SethTisue Unfortunately Scala 2.13.11-M1 does not solve the performance issue observed when building Flix.

In the following we specify two different build files when creating the archive file build/libs/flix.jar with the command gradle clean jar.
NB. Version 8.0.2 of Gradle is required (see issue 23962).

F:\flix>gradle -version | findstr Gradle
Gradle 8.0.2

F:\flix>gradle -b build.gradle clean jar

BUILD SUCCESSFUL in 2m 4s    <<<<<<<<<<<<<<<<<<<<<<<<<<<
3 actionable tasks: 3 executed
F:\flix>gradle -b build1.gradle clean jar

BUILD SUCCESSFUL in 7m 25s   <<<<<<<<<<<<<<<<<<<<<<<<<<<
3 actionable tasks: 3 executed

F:\flix>diff build.gradle build1.gradle
15,16c15,16
<     implementation 'org.scala-lang:scala-library:2.13.5!!'
<     implementation 'org.scala-lang:scala-reflect:2.13.5!!'
---
>     implementation files('lib/scala-library.jar')
>     implementation files('lib/scala-reflect.jar')
41a42
>             "-release:11",

NB. For completeness the contents of library.properties in archive lib/scala-library.jar (2.13.11-M1) is the following :

copyright.string=Copyright 2002-2023, LAMP/EPFL and Lightbend, Inc.
maven.version.number=2.13.11-bin-SNAPSHOT
osgi.version.number=2.13.11.v20230306-195001-unknown
shell.banner=%n     ________ ___   / /  ___%n    / __/ __// _ | / /  / _ |%n  __\\ \\/ /__/ __ |/ /__/ __ |%n /____/\\___/_/ |_/____/_/ | |%n                          |/  %s
version.number=2.13.11-20230306-195001-unknown

[KisaragiEffective]

Seems Logic.scala is very slow, especially PropositionalLogic. See https://github.com/KisaragiEffective/flix-profile/blob/master/snapshot-1679895243494.nps (load it in visualvm)

I think the root cause is neither ListSet or LinkedHashSet does not override addAll (which is used heavily in patmat) to pre-allocate their tables, causing expand them at random. --> filed as scala/bug#12760