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

No per-run caches in GenASM? #7422

Closed
scabug opened this issue Apr 25, 2013 · 11 comments
Closed

No per-run caches in GenASM? #7422

scabug opened this issue Apr 25, 2013 · 11 comments
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Apr 25, 2013

The compiler shouldn't have any global maps, and this one can trigger crashes in the presentation compiler. The smug message in the assertion is not very helpful for tracking down this issue, but I'd start by fixing the map.

I see this in GenASM.scala:155

  // Don't put this in per run caches. Contains entries for classes as well as members.
  val javaNameCache = new mutable.WeakHashMap[Symbol, Name]() ++= List(
    NothingClass        -> binarynme.RuntimeNothing,
    RuntimeNothingClass -> binarynme.RuntimeNothing,
    NullClass           -> binarynme.RuntimeNull,
    RuntimeNullClass    -> binarynme.RuntimeNull
  )

and crash (probably caused by two symbols for the same class, coming from different compiler runs):

ava.lang.AssertionError: assertion failed: how can getCommonSuperclass() do its job if different class symbols get the same bytecode-level internal name: scala/reflect/internal/Trees/Tree
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaName(GenASM.scala:547)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:581)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:587)
	at scala.tools.eclipse.JVMUtils$class.javaType(JVMUtils.scala:22)
	at scala.tools.eclipse.ScalaPresentationCompiler.javaType(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaJavaMapper$class.mapParamTypeSignature(ScalaJavaMapper.scala:242)
	at scala.tools.eclipse.ScalaPresentationCompiler.mapParamTypeSignature(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:318)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:105)
@scabug
Copy link
Author

scabug commented Apr 25, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7422?orig=1
Reporter: @dragos
Assignee: @magarciaEPFL
Affected Versions: 2.10.0

@scabug
Copy link
Author

scabug commented Apr 25, 2013

@magarciaEPFL said:

probably caused by two symbols for the same class, coming from different compiler runs

GenASM clears it maps at the end of GenASM.AsmPhase.run(), once all classes have been processed (that's also where bytecodeWriter.close() takes place).

Looks like GenASM.javaNameCache should also be cleared there. That should take care of clearing it properly. Regarding populating it properly, that can be achieved upon:

  /** Create a new phase */
  override def newPhase(p: Phase): Phase = new AsmPhase(p)

by just moving the following two maps from their current location `` into GenASM.AsmpPhase:

  val javaNameCache = new mutable.WeakHashMap[Symbol, Name]() ++= List(
    NothingClass        -> binarynme.RuntimeNothing,
    RuntimeNothingClass -> binarynme.RuntimeNothing,
    NullClass           -> binarynme.RuntimeNull,
    RuntimeNullClass    -> binarynme.RuntimeNull
  )

  // unlike javaNameCache, reverseJavaName contains entries only for class symbols and their internal names.
  val reverseJavaName = mutable.Map.empty[String, Symbol] ++= List(
    binarynme.RuntimeNothing.toString() -> RuntimeNothingClass, // RuntimeNothingClass is the bytecode-level return type of Scala methods with Nothing return-type.
    binarynme.RuntimeNull.toString()    -> RuntimeNullClass
  )

The above can't fail, even if holding for too long onto instances of GenASM, GenASM.AsmPhase, etc.

@dragos, can you confirm whether it works for you?

@scabug
Copy link
Author

scabug commented Apr 25, 2013

@retronym said:
AFAIK, all caches should now be created with perRunCaches. These will be automatically cleared at the end of each Run.

@scabug
Copy link
Author

scabug commented Apr 25, 2013

@magarciaEPFL said:
The alternative to perRunCaches that I mentioned should serve the IDE well (a phase starts and ends within a Run, thus the caches in question will get populated and cleared within a Run, too). The reason I mentioned this "alternative" is because I (faintly remember) entries disappearing (like, you know, magic) before the Run was over (actually, before the AsmPhase was over). Thus my suggestion to Iulian to stay clear from that magic's reach. Oh well, now it sounds as if I believe there's some magic to perRunCaches.

@scabug
Copy link
Author

scabug commented Apr 26, 2013

@magarciaEPFL said:
Proposed fix:
magarciaEPFL/scala@master...backendish6

It would be great if @dragos could test it in the Eclipse setting.

@scabug
Copy link
Author

scabug commented Apr 26, 2013

@magarciaEPFL said:
Regarding the proposed fix above, I've run a successful nightly on it.

Once @dragos OKs the fix above for the interplay with the presentation compiler, I can submit a PR.

@scabug
Copy link
Author

scabug commented Apr 26, 2013

@magarciaEPFL said:
scala/scala#2453

@scabug
Copy link
Author

scabug commented Apr 28, 2013

@dragos said:
I'll have a look.

@scabug
Copy link
Author

scabug commented Apr 30, 2013

@magarciaEPFL said:
scala/scala@a50f5db

@scabug scabug closed this as completed Apr 30, 2013
@scabug
Copy link
Author

scabug commented May 1, 2013

@dragos said:

java.lang.AssertionError: assertion failed: Different class symbols have the same bytecode-level internal name:
     name: scala/tools/nsc/transform/patmat/MatchApproximation/MatchApproximator/Test
   oldsym: scala.tools.nsc.transform.patmat.MatchApproximation.MatchApproximator.Test
  tracked: scala.tools.nsc.transform.patmat.MatchApproximation.MatchApproximator.Test
              
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaName(GenASM.scala:553)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:595)
	at scala.tools.nsc.backend.jvm.GenASM$JBuilder.javaType(GenASM.scala:601)
	at scala.tools.eclipse.JVMUtils$class.javaType(JVMUtils.scala:22)
	at scala.tools.eclipse.ScalaPresentationCompiler.javaType(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaJavaMapper$class.mapParamTypeSignature(ScalaJavaMapper.scala:242)
	at scala.tools.eclipse.ScalaPresentationCompiler.mapParamTypeSignature(ScalaPresentationCompiler.scala:36)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$$anonfun$13.apply(ScalaStructureBuilder.scala:659)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:244)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.collection.TraversableLike$class.map(TraversableLike.scala:244)
	at scala.collection.AbstractTraversable.map(Traversable.scala:103)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$class.addDef(ScalaStructureBuilder.scala:659)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$Builder.addDef(ScalaStructureBuilder.scala:857)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$DefOwner$class.addDef(ScalaStructureBuilder.scala:644)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$Builder.addDef(ScalaStructureBuilder.scala:857)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:929)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser$$anonfun$traverse$1.apply(ScalaStructureBuilder.scala:936)
	at scala.collection.immutable.List.foreach(List.scala:301)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser$TreeTraverser.traverse(ScalaStructureBuilder.scala:936)
	at scala.tools.eclipse.javaelements.ScalaStructureBuilder$StructureBuilderTraverser.traverse(ScalaStructureBuilder.scala:889)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply$mcV$sp(ScalaCompilationUnit.scala:89)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply(ScalaCompilationUnit.scala:88)
	at scala.tools.eclipse.javaelements.ScalaCompilationUnit$$anonfun$buildStructure$2$$anonfun$apply$5$$anonfun$apply$1.apply(ScalaCompilationUnit.scala:88)
	at scala.tools.nsc.util.InterruptReq.execute(InterruptReq.scala:26)
	at scala.tools.nsc.interactive.Global.pollForWork(Global.scala:423)
	at scala.tools.nsc.interactive.PresentationCompilerThread.run(PresentationCompilerThread.scala:22)

(this was before the latest nightly, I can't yet test it because the signature change in GenASM makes it hard to have the same source in 2.10 and 2.11)

@scabug
Copy link
Author

scabug commented May 1, 2013

@magarciaEPFL said:

Some context on the check in question: duplicates are detected to avoid the situation where, given a bytecode-level classname, the super classes tracked for it don't match those of "the other" symbol that also maps to the same internal name. Knowing a class' superclasses is needed to compute Java 6 StackMapTables.

Coming back to debugging, what we don't know is the relative ordering of:

(1) clearing and initial population of javaNameCache, reverseJavaName, ie

    javaNameCache.clear()
    javaNameCache   ++= List( ...

    reverseJavaName.clear()
    reverseJavaName ++= List( ...

(2) clearing of the above performed on Run end by perRunCaches

(3) adding symbols to the maps above, this is expected to happen for a single Run bracketed between (1) and (2).

From what I can see, two scenarios under which the assertion fails are:

(a) symbols from two (consecutive, in general different) Runs get in the same bracket (1) -- (2)

(b) within a single Run, two different class symbols map to the same bytecode-level internal name.

(c) both of the above? (not so likely)

Depending on which scenario is at play, is the party to blame.

@scabug scabug added this to the 2.11.0-M3 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant