Skip to content

Commit 30997e4

Browse files
retronymadriaanm
authored andcommitted
Backport ASM 6.2 upgrade to 2.11.x via 2.12.x (scala#6733)
Avoid performance problem after ASM upgrade in prod/cons analysis ASM 6.2 now creates a new Frame inside the loop in which `newExceptionValue` is called. We were including this frame in the case-class equality of the pseudo-instruction, `ExceptionProducer`, and upon receiving new instances each time the `ProdCons` analysis massively slowed down. This commit just captures the data we need: the stack top of the handler frame. Upgrade to scala-asm 6.2 See: scala/scala-asm#5 Upstream changes in ASM: scala/scala-asm@ASM_6_0...ASM_6_2 http://asm.ow2.io/versions.html The motivations, other than just keeping current, are: - support for Java 9/10/11 updates to the classfile format. - reducing needless String => Array[Char] conversions thanks to internal changes in ASM. This PR will fail to build until we publish artifact from scala/scala-asm. Includes a workaround for scala/bug#10418 Move to the standard way of defining a custom asm.Attribute It seems we don't need CustomAttr in our fork of scala-asm, we can just override Attribute.write. Customise label handling without needing to modify ASM directly Comment on our customizations to asm.tree.*Node (cherry picked from commit 79b7f2a)
1 parent bf79ccd commit 30997e4

File tree

15 files changed

+271
-18
lines changed

15 files changed

+271
-18
lines changed

src/compiler/scala/tools/nsc/backend/jvm/AsmUtils.scala

+42
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,48 @@ object AsmUtils {
5555
node
5656
}
5757

58+
def readClass(filename: String): ClassNode = readClass(classBytes(filename))
59+
60+
def classBytes(file: String): Array[Byte] = {
61+
val f = new java.io.RandomAccessFile(file, "r")
62+
val bytes = new Array[Byte](f.length.toInt)
63+
f.read(bytes)
64+
bytes
65+
}
66+
67+
def classFromBytes(bytes: Array[Byte]): ClassNode = {
68+
val node = new ClassNode1()
69+
new ClassReader(bytes).accept(node, ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES)
70+
71+
node
72+
}
73+
74+
// def main(args: Array[String]): Unit = println(textify(sortedClassRead(classBytes(args.head))))
75+
76+
def sortClassMembers(node: ClassNode): node.type = {
77+
node.fields.sort(_.name compareTo _.name)
78+
node.methods.sort(_.name compareTo _.name)
79+
node
80+
}
81+
82+
// drop ScalaSig annotation and class attributes
83+
def zapScalaClassAttrs(node: ClassNode): node.type = {
84+
if (node.visibleAnnotations != null)
85+
node.visibleAnnotations = node.visibleAnnotations.asScala.filterNot(a => a == null || a.desc.contains("Lscala/reflect/ScalaSignature")).asJava
86+
87+
node.attrs = null
88+
node
89+
}
90+
91+
def main(args: Array[String]): Unit = args.par.foreach { classFileName =>
92+
val node = zapScalaClassAttrs(sortClassMembers(classFromBytes(classBytes(classFileName))))
93+
94+
val pw = new PrintWriter(classFileName + ".asm")
95+
val trace = new TraceClassVisitor(pw)
96+
node.accept(trace)
97+
pw.close()
98+
}
99+
58100
/**
59101
* Returns a human-readable representation of the cnode ClassNode.
60102
*/

src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala

+10-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import scala.collection.mutable
1212
import scala.tools.nsc.io.AbstractFile
1313
import GenBCode._
1414
import BackendReporting._
15+
import scala.tools.asm.ClassWriter
1516

1617
/*
1718
* Traits encapsulating functionality to convert Scala AST Trees into ASM ClassNodes.
@@ -244,9 +245,14 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
244245
* can-multi-thread
245246
*/
246247
def createJAttribute(name: String, b: Array[Byte], offset: Int, len: Int): asm.Attribute = {
247-
val dest = new Array[Byte](len)
248-
System.arraycopy(b, offset, dest, 0, len)
249-
new asm.CustomAttr(name, dest)
248+
new asm.Attribute(name) {
249+
override def write(classWriter: ClassWriter, code: Array[Byte],
250+
codeLength: Int, maxStack: Int, maxLocals: Int): asm.ByteVector = {
251+
val byteVector = new asm.ByteVector(len)
252+
byteVector.putByteArray(b, offset, len)
253+
byteVector
254+
}
255+
}
250256
}
251257

252258
/*
@@ -766,7 +772,7 @@ abstract class BCodeHelpers extends BCodeIdiomatic with BytecodeWriters {
766772
this.cunit = cunit
767773

768774
val bType = mirrorClassClassBType(moduleClass)
769-
val mirrorClass = new asm.tree.ClassNode
775+
val mirrorClass = new ClassNode1
770776
mirrorClass.visit(
771777
classfileVersion,
772778
bType.info.get.flags,

src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
104104

105105
val classBType = classBTypeFromSymbol(claszSymbol)
106106

107-
cnode = new asm.tree.ClassNode()
107+
cnode = new ClassNode1()
108108

109109
initJClass(cnode)
110110

src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala

+4-2
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ abstract class BTypes {
9797
/**
9898
* Obtain the BType for a type descriptor or internal name. For class descriptors, the ClassBType
9999
* is constructed by parsing the corresponding classfile.
100-
*
100+
*
101101
* Some JVM operations use either a full descriptor or only an internal name. Example:
102102
* ANEWARRAY java/lang/String // a new array of strings (internal name for the String class)
103103
* ANEWARRAY [Ljava/lang/String; // a new array of array of string (full descriptor for the String class)
@@ -964,6 +964,8 @@ abstract class BTypes {
964964
// finds the first common one.
965965
// MOST LIKELY the answer can be found here, see the comments and links by Miguel:
966966
// - https://issues.scala-lang.org/browse/SI-3872
967+
// @jz Wouldn't it be better to walk the superclass chain of both types in reverse (starting from Object), and
968+
// finding the last common link? That would be O(N), whereas this looks O(N^2)
967969
firstCommonSuffix(this :: this.superClassesTransitive.orThrow, other :: other.superClassesTransitive.orThrow)
968970
}
969971

@@ -1155,4 +1157,4 @@ object BTypes {
11551157
// no static way (without symbol table instance) to get to nme.ScalaATTR / ScalaSignatureATTR
11561158
val ScalaAttributeName = "Scala"
11571159
val ScalaSigAttributeName = "ScalaSig"
1158-
}
1160+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/* NSC -- new Scala compiler
2+
* Copyright 2018 LAMP/EPFL
3+
* @author Martin Odersky
4+
*/
5+
package scala.tools.nsc.backend.jvm;
6+
7+
import scala.tools.asm.MethodVisitor;
8+
import scala.tools.asm.Opcodes;
9+
import scala.tools.asm.tree.ClassNode;
10+
import scala.tools.asm.tree.MethodNode;
11+
12+
/**
13+
* A subclass of {@link ClassNode} to customize the representation of
14+
* label nodes with {@link LabelNode1}.
15+
*/
16+
public class ClassNode1 extends ClassNode {
17+
public ClassNode1() {
18+
this(Opcodes.ASM6);
19+
}
20+
21+
public ClassNode1(int api) {
22+
super(api);
23+
}
24+
25+
@Override
26+
public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) {
27+
MethodNode method = new MethodNode1(access, name, descriptor, signature, exceptions);
28+
methods.add(method);
29+
return method;
30+
}
31+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/* NSC -- new Scala compiler
2+
* Copyright 2018 LAMP/EPFL
3+
* @author Martin Odersky
4+
*/
5+
package scala.tools.nsc.backend.jvm;
6+
7+
import scala.tools.asm.Label;
8+
import scala.tools.asm.tree.ClassNode;
9+
import scala.tools.asm.tree.LabelNode;
10+
11+
/**
12+
* A subclass of {@link LabelNode} to add user-definable flags.
13+
*/
14+
public class LabelNode1 extends LabelNode {
15+
public LabelNode1() {
16+
}
17+
18+
public LabelNode1(Label label) {
19+
super(label);
20+
}
21+
22+
public int flags;
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/* NSC -- new Scala compiler
2+
* Copyright 2018 LAMP/EPFL
3+
* @author Martin Odersky
4+
*/
5+
package scala.tools.nsc.backend.jvm;
6+
7+
import scala.tools.asm.Label;
8+
import scala.tools.asm.Opcodes;
9+
import scala.tools.asm.tree.LabelNode;
10+
import scala.tools.asm.tree.MethodNode;
11+
/**
12+
* A subclass of {@link MethodNode} to customize the representation of
13+
* label nodes with {@link LabelNode1}.
14+
*/
15+
public class MethodNode1 extends MethodNode {
16+
public MethodNode1(int api, int access, String name, String descriptor, String signature, String[] exceptions) {
17+
super(api, access, name, descriptor, signature, exceptions);
18+
}
19+
20+
public MethodNode1(int access, String name, String descriptor, String signature, String[] exceptions) {
21+
this(Opcodes.ASM6, access, name, descriptor, signature, exceptions);
22+
}
23+
24+
public MethodNode1(int api) {
25+
super(api);
26+
}
27+
28+
public MethodNode1() {
29+
this(Opcodes.ASM6);
30+
}
31+
32+
@Override
33+
protected LabelNode getLabelNode(Label label) {
34+
if (!(label.info instanceof LabelNode)) {
35+
label.info = new LabelNode1(label);
36+
}
37+
return (LabelNode) label.info;
38+
}
39+
}

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

Whitespace-only changes.

src/compiler/scala/tools/nsc/backend/jvm/analysis/ProdConsAnalyzer.scala

+14-8
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,13 @@ class ProdConsAnalyzer(methodNode: MethodNode, classInternalName: InternalName)
102102
inputValues(insn).iterator.flatMap(v => v.insns.asScala).toSet
103103
}
104104

105-
def consumersOfOutputsFrom(insn: AbstractInsnNode): Set[AbstractInsnNode] =
106-
_consumersOfOutputsFrom.get(insn).map(v => v.indices.flatMap(v.apply)(collection.breakOut): Set[AbstractInsnNode]).getOrElse(Set.empty)
105+
def consumersOfOutputsFrom(insn: AbstractInsnNode): Set[AbstractInsnNode] = insn match {
106+
case _: UninitializedLocalProducer => Set.empty
107+
case ParameterProducer(local) => consumersOfValueAt(methodNode.instructions.getFirst, local)
108+
case ExceptionProducer(handlerLabel, handlerStackTop) => consumersOfValueAt(handlerLabel, handlerStackTop)
109+
case _ =>
110+
_consumersOfOutputsFrom.get(insn).map(v => v.indices.flatMap(v.apply)(collection.breakOut): Set[AbstractInsnNode]).getOrElse(Set.empty)
111+
}
107112

108113
/**
109114
* Returns the potential initial producer instructions of a value in the frame of `insn`.
@@ -386,7 +391,7 @@ class ProdConsAnalyzer(methodNode: MethodNode, classInternalName: InternalName)
386391
private def outputValueSlots(insn: AbstractInsnNode): Seq[Int] = insn match {
387392
case ParameterProducer(local) => Seq(local)
388393
case UninitializedLocalProducer(local) => Seq(local)
389-
case ExceptionProducer(frame) => Seq(frame.stackTop)
394+
case ExceptionProducer(_, stackTop) => Seq(stackTop)
390395
case _ =>
391396
if (insn.getOpcode == -1) return Seq.empty
392397
if (isStore(insn)) {
@@ -459,11 +464,11 @@ abstract class InitialProducer extends AbstractInsnNode(-1) {
459464
override def accept(cv: MethodVisitor): Unit = throw new UnsupportedOperationException
460465
}
461466

462-
case class ParameterProducer(local: Int) extends InitialProducer
463-
case class UninitializedLocalProducer(local: Int) extends InitialProducer
464-
case class ExceptionProducer(handlerFrame: Frame[_ <: Value]) extends InitialProducer
467+
case class ParameterProducer(local: Int) extends InitialProducer
468+
case class UninitializedLocalProducer(local: Int) extends InitialProducer
469+
case class ExceptionProducer[V <: Value](handlerLabel: LabelNode, handlerStackTop: Int) extends InitialProducer
465470

466-
class InitialProducerSourceInterpreter extends SourceInterpreter {
471+
class InitialProducerSourceInterpreter extends SourceInterpreter(scala.tools.asm.Opcodes.ASM7_EXPERIMENTAL) {
467472
override def newParameterValue(isInstanceMethod: Boolean, local: Int, tp: Type): SourceValue = {
468473
new SourceValue(tp.getSize, ParameterProducer(local))
469474
}
@@ -473,6 +478,7 @@ class InitialProducerSourceInterpreter extends SourceInterpreter {
473478
}
474479

475480
override def newExceptionValue(tryCatchBlockNode: TryCatchBlockNode, handlerFrame: Frame[_ <: Value], exceptionType: Type): SourceValue = {
476-
new SourceValue(1, ExceptionProducer(handlerFrame))
481+
val handlerStackTop = handlerFrame.stackTop + 1 // +1 because this value is about to be pushed onto `handlerFrame`.
482+
new SourceValue(1, ExceptionProducer(tryCatchBlockNode.handler, handlerStackTop))
477483
}
478484
}

src/compiler/scala/tools/nsc/backend/jvm/opt/ByteCodeRepository.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class ByteCodeRepository(val classPath: ClassFileLookup[AbstractFile], val isJav
137137
private def parseClass(internalName: InternalName): Either[ClassNotFound, ClassNode] = {
138138
val fullName = internalName.replace('/', '.')
139139
classPath.findClassFile(fullName) map { classFile =>
140-
val classNode = new asm.tree.ClassNode()
140+
val classNode = new ClassNode1
141141
val classReader = new asm.ClassReader(classFile.toByteArray)
142142

143143
// Passing the InlineInfoAttributePrototype makes the ClassReader invoke the specific `read`

src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala

+1-1
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ object BytecodeUtils {
283283
*/
284284
def newLabelNode: LabelNode = {
285285
val label = new Label
286-
val labelNode = new LabelNode(label)
286+
val labelNode = new LabelNode1(label)
287287
label.info = labelNode
288288
labelNode
289289
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/* NEST (New Scala Test)
2+
* Copyright 2007-2013 LAMP/EPFL
3+
* @author Paul Phillips
4+
*/
5+
package scala.tools.partest
6+
package nest
7+
8+
import java.io.{Console => _, _}
9+
import java.nio.charset.Charset
10+
11+
object StreamCapture {
12+
def savingSystem[T](body: => T): T = {
13+
val savedOut = System.out
14+
val savedErr = System.err
15+
try body
16+
finally {
17+
System setErr savedErr
18+
System setOut savedOut
19+
}
20+
}
21+
22+
def capturingOutErr[A](output: OutputStream)(f: => A): A = {
23+
import java.io._
24+
val charset = Charset.defaultCharset()
25+
val printStream = new PrintStream(output, true, charset.name())
26+
savingSystem {
27+
System.setOut(printStream)
28+
System.setErr(printStream)
29+
try {
30+
scala.Console.withErr(printStream) {
31+
scala.Console.withOut(printStream) {
32+
f
33+
}
34+
}
35+
} finally {
36+
printStream.close()
37+
}
38+
}
39+
}
40+
41+
def withExtraProperties[A](extra: Map[String, String])(action: => A): A = {
42+
val saved = System.getProperties()
43+
val modified = new java.util.Properties()
44+
// on Java 9, we need to cast our way around this:
45+
// src/main/scala/scala/tools/partest/nest/StreamCapture.scala:44: ambiguous reference to overloaded definition,
46+
// both method putAll in class Properties of type (x$1: java.util.Map[_, _])Unit
47+
// and method putAll in class Hashtable of type (x$1: java.util.Map[_ <: Object, _ <: Object])Unit
48+
// match argument types (java.util.Properties)
49+
(modified: java.util.Hashtable[AnyRef, AnyRef]).putAll(saved)
50+
extra.foreach { case (k, v) => modified.setProperty(k, v) }
51+
// Trying to avoid other threads seeing the new properties object prior to the new entries
52+
// https://github.com/scala/scala/pull/6391#issuecomment-371346171
53+
UnsafeAccess.U.storeFence()
54+
System.setProperties(modified)
55+
try {
56+
action
57+
} finally {
58+
System.setProperties(saved)
59+
}
60+
}
61+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package scala.tools.nsc
2+
package backend.jvm
3+
4+
import java.util.concurrent.TimeUnit
5+
6+
import scala.tools.asm.tree.ClassNode
7+
import org.openjdk.jmh.annotations._
8+
import org.openjdk.jmh.infra.Blackhole
9+
10+
import scala.collection.JavaConverters.asScalaIteratorConverter
11+
import scala.tools.asm.tree.ClassNode
12+
13+
@BenchmarkMode(Array(Mode.AverageTime))
14+
@Fork(2)
15+
@Threads(1)
16+
@Warmup(iterations = 10)
17+
@Measurement(iterations = 10)
18+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
19+
@State(Scope.Benchmark)
20+
class ProdConsBenchmark {
21+
type G <: Global
22+
var global: G = _
23+
private var classNode: ClassNode = _
24+
25+
@Setup(Level.Trial) def setup(): Unit = {
26+
val settings = new Settings()
27+
settings.usejavacp.value = true
28+
val global = new Global(settings)
29+
import global._
30+
this.global = global.asInstanceOf[G]
31+
classNode = AsmUtils.readClass(global.classPath.findClassFile("scala.tools.nsc.typechecker.Implicits$ImplicitSearch").get.toByteArray)
32+
}
33+
34+
@Benchmark
35+
def prodCons(bh: Blackhole): Unit = {
36+
val global: G = this.global
37+
import global.genBCode.postProcessor.backendUtils._
38+
for (m <- classNode.methods.iterator().asScala) {
39+
bh.consume(new ProdConsAnalyzer(m, classNode.name))
40+
}
41+
}
42+
}
43+

test/junit/scala/tools/testing/BytecodeTesting.scala

Whitespace-only changes.

versions.properties

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ scala-swing.version.number=1.0.2
3333
akka-actor.version.number=2.3.16
3434
actors-migration.version.number=1.1.0
3535
jline.version=2.14.6
36-
scala-asm.version=6.0.0-scala-1
36+
scala-asm.version=6.2.0-scala-2
3737

3838
# external modules, used internally (not shipped)
3939
partest.version.number=1.0.16

0 commit comments

Comments
 (0)