8000 Backport ASM 6.2 upgrade to 2.11.x via 2.12.x (#6733) · adriaanm/scala@30997e4 · GitHub
[go: up one dir, main page]

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

Lines changed: 42 additions & 0 deletions
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

Lines changed: 10 additions & 4 deletions
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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 4 additions & 2 deletions
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+
}
Lines changed: 31 additions & 0 deletions
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+
}
Lines changed: 23 additions & 0 deletions
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+
}
Lines changed: 39 additions & 0 deletions
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

Lines changed: 14 additions & 8 deletions
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

Lines changed: 1 addition & 1 deletion
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`

0 commit comments

Comments
 (0)
0