For JDK 23 support, upgrade to ASM 9.7 #10744

SethTisue · 2024-04-09T19:00:09Z

looks like a safe upgrade for 2.13.14, knock on wood: https://asm.ow2.io/versions.html

@Philippus I did this myself (using your #10576 as a guide) because 2.13.14 is imminent, but I leave it to you to do Scala 2.12 and Scala 3, if you're available

SethTisue · 2024-04-09T19:38:46Z

[error] 	scala.tools.nsc.reporters.WConfTest
[error] 	scala.tools.nsc.backend.jvm.OptimizedBytecodeTest
[error] 	scala.tools.nsc.backend.jvm.opt.MethodLevelOptsTest

Philippus · 2024-04-09T19:54:46Z

Maybe it's a Jenkins thing? https://scala-ci.typesafe.com/job/scala-2.12.x-validate-main/5414/console says "No space left on device".

SethTisue · 2024-04-09T20:05:04Z

/rebuild

SethTisue · 2024-04-09T20:05:21Z

Jenkins did run out of inodes, but I'm able to reproduce the test failures locally.

SethTisue · 2024-04-09T21:00:25Z

extracting one of the failing tests,

% cat S.scala
class C {
  def t(as: Listt): Unit = {
    map(as, (_: Any) => return)
  }
  final def map(x: Listt, f: Any => Any): Any = {
    if (x eq Nill) "" else f("")
  }
}
object Nill extends Listt
class Listt
% ~/scala.213/build/pack/bin/scalac '-opt:inline:**' S.scala
error: Error while emitting C
-1 is out of bounds (min 0, max 0)
1 error
% ~/scala.213/build/pack/bin/scalac S.scala

SethTisue · 2024-04-09T21:34:12Z

adding -Vdebug gets us a stack trace from where the error is actually occurring, namely at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$4(ProdConsAnalyzer.scala:421)

[running phase jvm on S.scala]
java.lang.IndexOutOfBoundsException: -1 is out of bounds (min 0, max 0)
	at scala.collection.immutable.Vector.ioob(Vector.scala:285)
	at scala.collection.immutable.Vector1.apply(Vector.scala:390)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$4(ProdConsAnalyzer.scala:421)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$4$adapted(ProdConsAnalyzer.scala:416)
	at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:619)
	at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:617)
	at scala.collection.AbstractIterable.foreach(Iterable.scala:935)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$3(ProdConsAnalyzer.scala:416)
	at scala.runtime.java8.JFunction1$mcVI$sp.apply(JFunction1$mcVI$sp.scala:18)
	at scala.collection.immutable.Range.foreach(Range.scala:190)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$2(ProdConsAnalyzer.scala:415)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.$anonfun$_consumersOfOutputsFrom$2$adapted(ProdConsAnalyzer.scala:413)
	at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:619)
	at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:617)
	at scala.collection.AbstractIterator.foreach(Iterator.scala:1303)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer._consumersOfOutputsFrom$lzycompute(ProdConsAnalyzer.scala:413)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer._consumersOfOutputsFrom(ProdConsAnalyzer.scala:410)
	at scala.tools.nsc.backend.jvm.analysis.ProdConsAnalyzer.consumersOfOutputsFrom(ProdConsAnalyzer.scala:103)
	at scala.tools.nsc.backend.jvm.opt.BoxUnbox$BoxKind$.checkInstanceCreation(BoxUnbox.scala:619)
	at scala.tools.nsc.backend.jvm.opt.BoxUnbox$PrimitiveBox$.checkPrimitiveBox(BoxUnbox.scala:704)
	at scala.tools.nsc.backend.jvm.opt.BoxUnbox$BoxKind$.valueCreationKind(BoxUnbox.scala:598)
	at scala.tools.nsc.backend.jvm.opt.BoxUnbox.boxUnboxElimination(BoxUnbox.scala:393)
	at scala.tools.nsc.backend.jvm.opt.LocalOpt.removalRound$2(LocalOpt.scala:306)
	at scala.tools.nsc.backend.jvm.opt.LocalOpt.methodOptimizations(LocalOpt.scala:380)
	at scala.tools.nsc.backend.jvm.opt.LocalOpt.$anonfun$methodOptimizations$1(LocalOpt.scala:215)
	at scala.tools.nsc.backend.jvm.opt.LocalOpt.$anonfun$methodOptimizations$1$adapted(LocalOpt.scala:214)
	at scala.collection.IterableOnceOps.foldLeft(IterableOnce.scala:727)
	at scala.collection.IterableOnceOps.foldLeft$(IterableOnce.scala:721)
	at scala.collection.AbstractIterable.foldLeft(Iterable.scala:935)
	at scala.tools.nsc.backend.jvm.opt.LocalOpt.methodOptimizations(LocalOpt.scala:214)
	at scala.tools.nsc.backend.jvm.PostProcessor.$anonfun$localOptimizations$1(PostProcessor.scala:131)
	at scala.runtime.java8.JFunction0$mcZ$sp.apply(JFunction0$mcZ$sp.scala:17)
	at scala.reflect.internal.util.Statistics.timed(Statistics.scala:309)
	at scala.tools.nsc.backend.jvm.PostProcessor.localOptimizations(PostProcessor.scala:131)
	at scala.tools.nsc.backend.jvm.PostProcessor.sendToDisk(PostProcessor.scala:65)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.$anonfun$postProcessUnit$3(GeneratedClassHandler.scala:131)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.$anonfun$postProcessUnit$3$adapted(GeneratedClassHandler.scala:131)
	at scala.collection.immutable.List.foreach(List.scala:334)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.$anonfun$postProcessUnit$2(GeneratedClassHandler.scala:131)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
	at scala.tools.nsc.backend.jvm.PostProcessorFrontendAccess$PostProcessorFrontendAccessImpl.withThreadLocalReporter(PostProcessorFrontendAccess.scala:242)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.$anonfun$postProcessUnit$1(GeneratedClassHandler.scala:130)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
	at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:687)
	at scala.concurrent.impl.Promise$Transformation.run(Promise.scala:467)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$SyncWritingClassHandler$$anonfun$$lessinit$greater$1.execute(GeneratedClassHandler.scala:185)
	at scala.concurrent.impl.ExecutionContextImpl.execute(ExecutionContextImpl.scala:21)
	at scala.concurrent.impl.Promise$Transformation.submitWithValue(Promise.scala:429)
	at scala.concurrent.impl.Promise$DefaultPromise.submitWithValue(Promise.scala:338)
	at scala.concurrent.impl.Promise$DefaultPromise.dispatchOrAddCallbacks(Promise.scala:312)
	at scala.concurrent.impl.Promise$DefaultPromise.map(Promise.scala:182)
	at scala.concurrent.Future$.apply(Future.scala:687)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.postProcessUnit(GeneratedClassHandler.scala:126)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$WritingClassHandler.process(GeneratedClassHandler.scala:119)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$GlobalOptimisingGeneratedClassHandler.$anonfun$complete$1(GeneratedClassHandler.scala:99)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$GlobalOptimisingGeneratedClassHandler.$anonfun$complete$1$adapted(GeneratedClassHandler.scala:99)
	at scala.collection.immutable.List.foreach(List.scala:334)
	at scala.tools.nsc.backend.jvm.GeneratedClassHandler$GlobalOptimisingGeneratedClassHandler.complete(GeneratedClassHandler.scala:99)
	at scala.tools.nsc.backend.jvm.GenBCode$BCodePhase.$anonfun$run$1(GenBCode.scala:81)
	at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.scala:18)
	at scala.reflect.internal.util.Statistics.timed(Statistics.scala:309)
	at scala.tools.nsc.backend.jvm.GenBCode$BCodePhase.run(GenBCode.scala:78)
	at scala.tools.nsc.Global$Run.compileUnitsInternal(Global.scala:1559)
	at scala.tools.nsc.Global$Run.compileUnits(Global.scala:1543)
	at scala.tools.nsc.Global$Run.compileSources(Global.scala:1535)
	at scala.tools.nsc.Global$Run.compile(Global.scala:1669)
	at scala.tools.nsc.Driver.doCompile(Driver.scala:48)
	at scala.tools.nsc.MainClass.doCompile(Main.scala:30)
	at scala.tools.nsc.Driver.process(Driver.scala:68)
	at scala.tools.nsc.Driver.main(Driver.scala:82)
	at scala.tools.nsc.Main.main(Main.scala)
error: Error while emitting C

SethTisue · 2024-04-09T21:37:33Z

looks like we should hold this for 2.13.15, which is fine... it's still really early days for JDK 23

SethTisue · 2024-04-09T21:38:23Z

@lrytz this is usually your wheelhouse, so no rush, but perhaps you'd like to try to figure out what this is about

som-snytt · 2024-04-09T23:38:03Z

Seth, I apologize for teasing you on another ticket as "risk-averse". Since the life-changing eclipse day, I see you're willing to just change the underpinnings of how the compiler emits bytecode at the eleventh hour, or five to midnight.

SethTisue · 2024-04-10T00:08:03Z

I eat risk for breakfast! But sometimes I skip breakfast.

som-snytt · 2024-04-10T01:24:34Z

Does anyone else read WConfTest and assume it has something to do with basketball? Like Western Conference Championship Test. I'll take a further look as a good first issue, if a certain someone doesn't jump all over it because they're relaxing after the effort to put out 2.13.14.

lrytz · 2024-04-15T15:00:01Z

Took a quick look, an index is off by one in ProdConsAnalyzer, the producer is an ExceptionProducer. Looking at the changelog, it's quite certainly caused by https://gitlab.ow2.org/asm/asm/-/merge_requests/388/diffs. I'll dig deeper tomorrow.

som-snytt · 2024-04-15T15:48:37Z

Actually I got that far the other day, but on the learning curve, I thought I'd stop and rest my eyes for a bit. I probably won't climb higher by tomorrow.

lrytz · 2024-04-16T14:38:24Z

https://gitlab.ow2.org/asm/asm/-/merge_requests/388/diffs#note_101668

lrytz · 2024-04-16T15:04:19Z

Hmm, needs more work

[error] java.lang.NullPointerException
[error] scala.tools.nsc.backend.jvm.opt.CallGraph.$anonfun$addMethod$1(CallGraph.scala:167)

The ASM analyzer changed in 9.7 to no longer call `clearStack()` before `newExceptionValue`. We need to get the index of the exception value, which is always the first slot after all locals.

lrytz · 2024-04-17T14:44:48Z

I think this time it's a bug in asm, published my fix attempt as 9.7.0-scala-2 (scala/scala-asm@cb80825)

lrytz · 2024-04-17T15:08:25Z

Even if this goes green, I worry about the performance impact of the ASM change (https://gitlab.ow2.org/asm/asm/-/merge_requests/388/diffs). IIUC it's one additional merge call for each instruction covered by an exception handler (one for each handler). I need to do some benchmarking.

lrytz · 2024-04-18T07:46:11Z

Performance looks fine, I compared the same compiler with asm 9.6 (60f4d19) and 9.7 (bf0dd33)

# scalap corpus

# old asm
[info] HotScalacBenchmark.compile                                         latest  -opt:inline:**       false  2.13.14-bin-60f4d19-SNAPSHOT    scalap  sample  427  732,721 ± 6,250  ms/op
# new asm
[info] HotScalacBenchmark.compile                                         latest  -opt:inline:**       false  2.13.14-bin-bf0dd33-SNAPSHOT    scalap  sample  418  739,404 ± 5,886  ms/op
# without optimizer
[info] HotScalacBenchmark.compile                                         latest                    false  2.13.14-bin-bf0dd33-SNAPSHOT    scalap  sample  802  381,399 ± 3,115  ms/op

# scala corpus

#old asm
[info] HotScalacBenchmark.compile                                         latest  -opt:inline:**       false  2.13.14-bin-60f4d19-SNAPSHOT     scala  sample   30  46607,106 ± 677,909  ms/op
# new asm
[info] HotScalacBenchmark.compile                                         latest  -opt:inline:**       false  2.13.14-bin-bf0dd33-SNAPSHOT     scala  sample   30  46911,333 ± 593,243  ms/op
# without optimizer
[info] HotScalacBenchmark.compile                                         latest                    false  2.13.14-bin-bf0dd33-SNAPSHOT     scala  sample   30  20785,852 ± 504,137  ms/op

My first fix in this PR (in newExceptionValue) seems fine. For the fix I did in 9.7.0-scala-2 (scala/scala-asm@cb80825), let's wait and see what the maintainers say.

lrytz · 2024-04-22T07:43:39Z

let's wait and see what the maintainers say

They actually fixed it upstream already (https://gitlab.ow2.org/asm/asm/-/merge_requests/390), but that's not released yet.

We can either wait with our ASM upgrade until 9.7.1 is out, or go ahead with my 9.7.0-scala-2 that includes an equivalent fix.

For JDK 23 support, upgrade to ASM 9.7

41ba962

SethTisue added the release-notes worth highlighting in next release notes label Apr 9, 2024

SethTisue added this to the 2.13.14 milestone Apr 9, 2024

SethTisue self-assigned this Apr 9, 2024

SethTisue added the prio:hi high priority (used only by core team, only near release time) label Apr 9, 2024

This was referenced Apr 9, 2024

Upgrade to asm 9.7, for JDK23 support #10745

Closed

[nomerge] Upgrade to asm 9.7, for JDK23 support #10747

Draft

SethTisue marked this pull request as draft April 9, 2024 19:38

SethTisue mentioned this pull request Apr 9, 2024

JDK 22 and 23 scala/docs.scala-lang#3005

Merged

SethTisue modified the milestones: 2.13.14, 2.13.15 Apr 9, 2024

SethTisue removed their assignment Apr 9, 2024

Philippus mentioned this pull request Apr 9, 2024

Update asm to 9.7 scala/scala3#20144

Draft

lrytz added 2 commits April 17, 2024 09:29

More robust way to get index of first stack value in optimizer

b0261c7

The ASM analyzer changed in 9.7 to no longer call `clearStack()` before `newExceptionValue`. We need to get the index of the exception value, which is always the first slot after all locals.

scala-asm 9.7.0-scala-2

bf0dd33

lrytz force-pushed the asm-9.7-scala-2.13.x branch from f2a771a to bf0dd33 Compare April 17, 2024 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

For JDK 23 support, upgrade to ASM 9.7 #10744

For JDK 23 support, upgrade to ASM 9.7 #10744

SethTisue commented Apr 9, 2024 •

edited

SethTisue commented Apr 9, 2024

Philippus commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024 •

edited

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

som-snytt commented Apr 9, 2024

SethTisue commented Apr 10, 2024 •

edited

som-snytt commented Apr 10, 2024 •

edited

lrytz commented Apr 15, 2024

som-snytt commented Apr 15, 2024

lrytz commented Apr 16, 2024

lrytz commented Apr 16, 2024

lrytz commented Apr 17, 2024

lrytz commented Apr 17, 2024

lrytz commented Apr 18, 2024

lrytz commented Apr 22, 2024

For JDK 23 support, upgrade to ASM 9.7 #10744

Are you sure you want to change the base?

For JDK 23 support, upgrade to ASM 9.7 #10744

Conversation

SethTisue commented Apr 9, 2024 • edited

SethTisue commented Apr 9, 2024

Philippus commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024 • edited

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

SethTisue commented Apr 9, 2024

som-snytt commented Apr 9, 2024

SethTisue commented Apr 10, 2024 • edited

som-snytt commented Apr 10, 2024 • edited

lrytz commented Apr 15, 2024

som-snytt commented Apr 15, 2024

lrytz commented Apr 16, 2024

lrytz commented Apr 16, 2024

lrytz commented Apr 17, 2024

lrytz commented Apr 17, 2024

lrytz commented Apr 18, 2024

lrytz commented Apr 22, 2024

SethTisue commented Apr 9, 2024 •

edited

SethTisue commented Apr 9, 2024 •

edited

SethTisue commented Apr 10, 2024 •

edited

som-snytt commented Apr 10, 2024 •

edited