From 880d304dde48ebc90063cd980ef43ca3076b4848 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 23 Aug 2020 11:56:03 +0200 Subject: [PATCH 1/4] Avoid some expensive collection operations in backend --- .../tools/backend/jvm/BCodeBodyBuilder.scala | 12 +++-- .../tools/backend/jvm/BCodeIdiomatic.scala | 8 ++-- .../tools/backend/jvm/BCodeSkelBuilder.scala | 12 ++--- .../src/dotty/tools/backend/jvm/BTypes.scala | 7 ++- .../tools/backend/jvm/BTypesFromSymbols.scala | 47 +++++++++---------- .../dotty/tools/backend/jvm/GenBCodeOps.scala | 3 +- 6 files changed, 46 insertions(+), 43 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index cb854c24568e..3bc859428ddb 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -1021,9 +1021,15 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { } } - def genLoadArguments(args: List[Tree], btpes: List[BType]): Unit = { - (args zip btpes) foreach { case (arg, btpe) => genLoad(arg, btpe) } - } + def genLoadArguments(args: List[Tree], btpes: List[BType]): Unit = + args match + case arg :: args1 => + btpes match + case btpe :: btpes1 => + genLoad(arg, btpe) + genLoadArguments(args1, btpes1) + case _ => + case _ => def genLoadModule(tree: Tree): BType = { val module = ( diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala index 708a6fee9fff..4112dcbeda20 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeIdiomatic.scala @@ -35,10 +35,10 @@ trait BCodeIdiomatic { lazy val majorVersion: Int = (classfileVersion & 0xFF) lazy val emitStackMapFrame = (majorVersion >= 50) - val extraProc: Int = GenBCodeOps.mkFlags( - asm.ClassWriter.COMPUTE_MAXS, - if (emitStackMapFrame) asm.ClassWriter.COMPUTE_FRAMES else 0 - ) + val extraProc: Int = + import GenBCodeOps.addFlagIf + asm.ClassWriter.COMPUTE_MAXS + .addFlagIf(emitStackMapFrame, asm.ClassWriter.COMPUTE_FRAMES) lazy val JavaStringBuilderClassName = jlStringBuilderRef.internalName diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 9a1bafa423c7..3dd7230b1875 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -690,12 +690,12 @@ trait BCodeSkelBuilder extends BCodeHelpers { val isNative = methSymbol.hasAnnotation(NativeAttr) val isAbstractMethod = (methSymbol.is(Deferred) || (methSymbol.owner.isInterface && ((methSymbol.is(Deferred)) || methSymbol.isClassConstructor))) - val flags = GenBCodeOps.mkFlags( - javaFlags(methSymbol), - if (isAbstractMethod) asm.Opcodes.ACC_ABSTRACT else 0, - if (false /*methSymbol.isStrictFP*/) asm.Opcodes.ACC_STRICT else 0, - if (isNative) asm.Opcodes.ACC_NATIVE else 0 // native methods of objects are generated in mirror classes - ) + val flags = + import GenBCodeOps.addFlagIf + javaFlags(methSymbol) + .addFlagIf(isAbstractMethod, asm.Opcodes.ACC_ABSTRACT) + .addFlagIf(false /*methSymbol.isStrictFP*/, asm.Opcodes.ACC_STRICT) + .addFlagIf(isNative, asm.Opcodes.ACC_NATIVE) // native methods of objects are generated in mirror classes // TODO needed? for(ann <- m.symbol.annotations) { ann.symbol.initialize } initJMethod(flags, params.map(p => p.symbol.annotations)) diff --git a/compiler/src/dotty/tools/backend/jvm/BTypes.scala b/compiler/src/dotty/tools/backend/jvm/BTypes.scala index b49b97c58ec9..fb399461a03c 100644 --- a/compiler/src/dotty/tools/backend/jvm/BTypes.scala +++ b/compiler/src/dotty/tools/backend/jvm/BTypes.scala @@ -649,14 +649,13 @@ abstract class BTypes { def innerClassAttributeEntry: Option[InnerClassEntry] = info.nestedInfo map { case NestedInfo(_, outerName, innerName, isStaticNestedClass) => + import GenBCodeOps.addFlagIf InnerClassEntry( internalName, outerName.orNull, innerName.orNull, - GenBCodeOps.mkFlags( - info.flags, - if (isStaticNestedClass) asm.Opcodes.ACC_STATIC else 0 - ) & ClassBType.INNER_CLASSES_FLAGS + info.flags.addFlagIf(isStaticNestedClass, asm.Opcodes.ACC_STATIC) + & ClassBType.INNER_CLASSES_FLAGS ) } diff --git a/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala b/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala index beea05e0dcc1..7fb716b00782 100644 --- a/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala +++ b/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala @@ -304,39 +304,36 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I) extends BTypes { val finalFlag = sym.is(Final) && !toDenot(sym).isClassConstructor && !sym.is(Mutable) && !sym.enclosingClass.is(Trait) import asm.Opcodes._ - GenBCodeOps.mkFlags( - if (privateFlag) ACC_PRIVATE else ACC_PUBLIC, - if (sym.is(Deferred) || sym.isOneOf(AbstractOrTrait)) ACC_ABSTRACT else 0, - if (sym.isInterface) ACC_INTERFACE else 0, - - if (finalFlag && + import GenBCodeOps.addFlagIf + 0 .addFlagIf(privateFlag, ACC_PRIVATE) + .addFlagIf(!privateFlag, ACC_PUBLIC) + .addFlagIf(sym.is(Deferred) || sym.isOneOf(AbstractOrTrait), ACC_ABSTRACT) + .addFlagIf(sym.isInterface, ACC_INTERFACE) + .addFlagIf(finalFlag // Primitives are "abstract final" to prohibit instantiation // without having to provide any implementations, but that is an // illegal combination of modifiers at the bytecode level so // suppress final if abstract if present. - !sym.isOneOf(AbstractOrTrait) && + && !sym.isOneOf(AbstractOrTrait) // Mixin forwarders are bridges and can be final, but final bridges confuse some frameworks - !sym.is(Bridge)) - ACC_FINAL else 0, - - if (sym.isStaticMember) ACC_STATIC else 0, - if (sym.is(Bridge)) ACC_BRIDGE | ACC_SYNTHETIC else 0, - if (sym.is(Artifact)) ACC_SYNTHETIC else 0, - if (sym.isClass && !sym.isInterface) ACC_SUPER else 0, - if (sym.isAllOf(JavaEnumTrait)) ACC_ENUM else 0, - if (sym.is(JavaVarargs)) ACC_VARARGS else 0, - if (sym.is(Synchronized)) ACC_SYNCHRONIZED else 0, - if (sym.isDeprecated) ACC_DEPRECATED else 0, - if (sym.is(Enum)) ACC_ENUM else 0 - ) + && !sym.is(Bridge), ACC_FINAL) + .addFlagIf(sym.isStaticMember, ACC_STATIC) + .addFlagIf(sym.is(Bridge), ACC_BRIDGE | ACC_SYNTHETIC) + .addFlagIf(sym.is(Artifact), ACC_SYNTHETIC) + .addFlagIf(sym.isClass && !sym.isInterface, ACC_SUPER) + .addFlagIf(sym.isAllOf(JavaEnumTrait), ACC_ENUM) + .addFlagIf(sym.is(JavaVarargs), ACC_VARARGS) + .addFlagIf(sym.is(Synchronized), ACC_SYNCHRONIZED) + .addFlagIf(sym.isDeprecated, ACC_DEPRECATED) + .addFlagIf(sym.is(Enum), ACC_ENUM) } def javaFieldFlags(sym: Symbol) = { import asm.Opcodes._ - javaFlags(sym) | GenBCodeOps.mkFlags( - if (sym.hasAnnotation(TransientAttr)) ACC_TRANSIENT else 0, - if (sym.hasAnnotation(VolatileAttr)) ACC_VOLATILE else 0, - if (sym.is(Mutable)) 0 else ACC_FINAL - ) + import GenBCodeOps.addFlagIf + javaFlags(sym) + .addFlagIf(sym.hasAnnotation(TransientAttr), ACC_TRANSIENT) + .addFlagIf(sym.hasAnnotation(VolatileAttr), ACC_VOLATILE) + .addFlagIf(!sym.is(Mutable), ACC_FINAL) } } diff --git a/compiler/src/dotty/tools/backend/jvm/GenBCodeOps.scala b/compiler/src/dotty/tools/backend/jvm/GenBCodeOps.scala index 884b86751b03..210e47566cb9 100644 --- a/compiler/src/dotty/tools/backend/jvm/GenBCodeOps.scala +++ b/compiler/src/dotty/tools/backend/jvm/GenBCodeOps.scala @@ -7,7 +7,8 @@ import scala.tools.asm object GenBCodeOps extends GenBCodeOps class GenBCodeOps { - def mkFlags(args: Int*) = args.foldLeft(0)(_ | _) + extension (flags: Int) + def addFlagIf(cond: Boolean, flag: Int): Int = if cond then flags | flag else flags final val PublicStatic = asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_STATIC final val PublicStaticFinal = asm.Opcodes.ACC_PUBLIC | asm.Opcodes.ACC_STATIC | asm.Opcodes.ACC_FINAL From d8555fdfa28e499a437d3d9fe7609383283868cf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 23 Aug 2020 12:38:21 +0200 Subject: [PATCH 2/4] Add nestedExists extension method Add nestedExists extension method for List[List[T]] exists and optimize it as well as nestedMap. --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 4 ++-- compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala | 2 +- compiler/src/dotty/tools/dotc/core/Decorators.scala | 8 ++++++-- compiler/src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- .../src/dotty/tools/dotc/transform/ElimRepeated.scala | 2 +- 6 files changed, 12 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 03b879699957..0df97a71e4e8 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -596,9 +596,9 @@ object desugar { (ordinalMethLit(ordinal) :: enumLabelLit(className.toString) :: Nil, scaffolding) else (Nil, Nil) def copyMeths = { - val hasRepeatedParam = constrVparamss.exists(_.exists { + val hasRepeatedParam = constrVparamss.nestedExists { case ValDef(_, tpt, _) => isRepeated(tpt) - }) + } if (mods.is(Abstract) || hasRepeatedParam) Nil // cannot have default arguments for repeated parameters, hence copy method is not issued else { def copyDefault(vparam: ValDef) = diff --git a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala index 4ed857678b26..74a169126eaa 100644 --- a/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala +++ b/compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala @@ -297,7 +297,7 @@ object DesugarEnums { case parent => parent.isType && typeHasRef(parent) } - vparamss.exists(_.exists(valDefHasRef)) || parents.exists(parentHasRef) + vparamss.nestedExists(valDefHasRef) || parents.exists(parentHasRef) } /** A pair consisting of diff --git a/compiler/src/dotty/tools/dotc/core/Decorators.scala b/compiler/src/dotty/tools/dotc/core/Decorators.scala index 8a9ed11fca03..7e6fb0c7f252 100644 --- a/compiler/src/dotty/tools/dotc/core/Decorators.scala +++ b/compiler/src/dotty/tools/dotc/core/Decorators.scala @@ -195,12 +195,16 @@ object Decorators { } extension [T, U](xss: List[List[T]]) - def nestedMap(f: T => U): List[List[U]] = - xss.map(_.map(f)) + def nestedMap(f: T => U): List[List[U]] = xss match + case xs :: xss1 => xs.map(f) :: xss1.nestedMap(f) + case nil => Nil def nestedMapConserve(f: T => U): List[List[U]] = xss.mapconserve(_.mapconserve(f)) def nestedZipWithConserve(yss: List[List[U]])(f: (T, U) => T): List[List[T]] = xss.zipWithConserve(yss)((xs, ys) => xs.zipWithConserve(ys)(f)) + def nestedExists(p: T => Boolean): Boolean = xss match + case xs :: xss1 => xs.exists(p) || xss1.nestedExists(p) + case nil => false end extension extension (text: Text) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index ce5235720a07..44e9d162d8a6 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -899,7 +899,7 @@ object SymDenotations { else if is(NoDefaultParams) then false else val result = - rawParamss.exists(_.exists(_.is(HasDefault))) + rawParamss.nestedExists(_.is(HasDefault)) || allOverriddenSymbols.exists(_.hasDefaultParams) setFlag(if result then HasDefaultParams else NoDefaultParams) result diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 5aba3e43e368..f31dfe675d88 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -3856,7 +3856,7 @@ object Parsers { val problem = tree match case tree: MemberDef if !(tree.mods.flags & ModifierFlags).isEmpty => i"refinement cannot be ${(tree.mods.flags & ModifierFlags).flagStrings().mkString("`", "`, `", "`")}" - case tree: DefDef if tree.vparamss.exists(_.exists(!_.rhs.isEmpty)) => + case tree: DefDef if tree.vparamss.nestedExists(!_.rhs.isEmpty) => i"refinement cannot have default arguments" case tree: ValOrDefDef => if tree.rhs.isEmpty then "" diff --git a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala index 85e2a62c9172..ecf2995cf30a 100644 --- a/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala +++ b/compiler/src/dotty/tools/dotc/transform/ElimRepeated.scala @@ -237,7 +237,7 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase => /** Is there a repeated parameter in some parameter list? */ private def hasRepeatedParams(sym: Symbol)(using Context): Boolean = - sym.info.paramInfoss.flatten.exists(_.isRepeatedParam) + sym.info.paramInfoss.nestedExists(_.isRepeatedParam) /** Is this the type of a method that has a repeated parameter type as * its last parameter in the last parameter list? From 84f7a77a064d3f9700d97fb800afa4d1e11fae8a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 24 Aug 2020 19:02:43 +0200 Subject: [PATCH 3/4] Reduce SourcePosition allocations --- compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala | 2 +- compiler/src/dotty/tools/dotc/util/SourcePosition.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala index 3dd7230b1875..2c575309d9f1 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeSkelBuilder.scala @@ -535,7 +535,7 @@ trait BCodeSkelBuilder extends BCodeHelpers { } def lineNumber(tree: Tree): Unit = { if (!emitLines || !tree.span.exists) return; - val nr = ctx.source.atSpan(tree.span).line + 1 + val nr = ctx.source.offsetToLine(tree.span.point) + 1 if (nr != lastEmittedLineNr) { lastEmittedLineNr = nr lastInsn match { diff --git a/compiler/src/dotty/tools/dotc/util/SourcePosition.scala b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala index 79919442a3b2..35d16cff9f44 100644 --- a/compiler/src/dotty/tools/dotc/util/SourcePosition.scala +++ b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala @@ -25,7 +25,7 @@ extends SrcPos, interfaces.SourcePosition, Showable { def point: Int = span.point - def line: Int = if (source.content().length != 0) source.offsetToLine(point) else -1 + def line: Int = if (source.length != 0) source.offsetToLine(point) else -1 /** Extracts the lines from the underlying source file as `Array[Char]`*/ def linesSlice: Array[Char] = From b0cfcccf206c13def15132baa83b3a39439b3baf Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 24 Aug 2020 15:38:06 +0200 Subject: [PATCH 4/4] Generate DottyPrimitives only once --- .../src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala | 2 +- compiler/src/dotty/tools/backend/jvm/GenBCode.scala | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala index 3bc859428ddb..e4a0e282a5a8 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeBodyBuilder.scala @@ -39,7 +39,7 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder { import coreBTypes._ import BCodeBodyBuilder._ - private val primitives = new DottyPrimitives(ctx) + protected val primitives: DottyPrimitives /* * Functionality to build the body of ASM MethodNode, except for `synchronized` and `try` expressions. diff --git a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala index 181e4c18f452..efc0256af177 100644 --- a/compiler/src/dotty/tools/backend/jvm/GenBCode.scala +++ b/compiler/src/dotty/tools/backend/jvm/GenBCode.scala @@ -50,9 +50,13 @@ class GenBCode extends Phase { myOutput } + private var myPrimitives: DottyPrimitives = null + def run(using Context): Unit = - GenBCodePipeline( - DottyBackendInterface(outputDir, superCallsMap) + if myPrimitives == null then myPrimitives = new DottyPrimitives(ctx) + new GenBCodePipeline( + DottyBackendInterface(outputDir, superCallsMap), + myPrimitives ).run(ctx.compilationUnit.tpdTree) @@ -74,7 +78,7 @@ object GenBCode { val name: String = "genBCode" } -class GenBCodePipeline(val int: DottyBackendInterface)(using Context) extends BCodeSyncAndTry { +class GenBCodePipeline(val int: DottyBackendInterface, val primitives: DottyPrimitives)(using Context) extends BCodeSyncAndTry { import DottyBackendInterface.symExtensions private var tree: Tree = _