Skip to content

Commit 79e7334

Browse files
adriaanmadriaanm
committed
Merge pull request scala#5202 from lrytz/lineNumbers
Keep line numbers when inlining from the same compilation unit
2 parents 28e0e18 + a1ea0aa commit 79e7334

File tree

16 files changed

+175
-109
lines changed

16 files changed

+175
-109
lines changed

‎src/compiler/scala/tools/nsc/backend/jvm/GenBCode.scala

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,16 @@ abstract class GenBCode extends BCodeSyncAndTry {
7777

7878
/* ---------------- q2 ---------------- */
7979

80-
case class Item2(arrivalPos: Int,
81-
mirror: asm.tree.ClassNode,
82-
plain: asm.tree.ClassNode,
83-
bean: asm.tree.ClassNode,
84-
outFolder: scala.tools.nsc.io.AbstractFile) {
80+
case class Item2(arrivalPos: Int,
81+
mirror: asm.tree.ClassNode,
82+
plain: asm.tree.ClassNode,
83+
bean: asm.tree.ClassNode,
84+
sourceFilePath: String,
85+
outFolder: scala.tools.nsc.io.AbstractFile) {
8586
def isPoison = { arrivalPos == Int.MaxValue }
8687
}
8788

88-
private val poison2 = Item2(Int.MaxValue, null, null, null, null)
89+
private val poison2 = Item2(Int.MaxValue, null, null, null, null, null)
8990
private val q2 = new _root_.java.util.LinkedList[Item2]
9091

9192
/* ---------------- q3 ---------------- */
@@ -205,6 +206,7 @@ abstract class GenBCode extends BCodeSyncAndTry {
205206
val item2 =
206207
Item2(arrivalPos,
207208
mirrorC, plainC, beanC,
209+
cunit.source.file.canonicalPath,
208210
outF)
209211

210212
q2 add item2 // at the very end of this method so that no Worker2 thread starts mutating before we're done.
@@ -226,10 +228,11 @@ abstract class GenBCode extends BCodeSyncAndTry {
226228
// add classes to the bytecode repo before building the call graph: the latter needs to
227229
// look up classes and methods in the code repo.
228230
if (settings.optAddToBytecodeRepository) q2.asScala foreach {
229-
case Item2(_, mirror, plain, bean, _) =>
230-
if (mirror != null) byteCodeRepository.add(mirror, ByteCodeRepository.CompilationUnit)
231-
if (plain != null) byteCodeRepository.add(plain, ByteCodeRepository.CompilationUnit)
232-
if (bean != null) byteCodeRepository.add(bean, ByteCodeRepository.CompilationUnit)
231+
case Item2(_, mirror, plain, bean, sourceFilePath, _) =>
232+
val someSourceFilePath = Some(sourceFilePath)
233+
if (mirror != null) byteCodeRepository.add(mirror, someSourceFilePath)
234+
if (plain != null) byteCodeRepository.add(plain, someSourceFilePath)
235+
if (bean != null) byteCodeRepository.add(bean, someSourceFilePath)
233236
}
234237
if (settings.optBuildCallGraph) q2.asScala foreach { item =>
235238
// skip call graph for mirror / bean: wd don't inline into tem, and they are not used in the plain class
@@ -286,7 +289,7 @@ abstract class GenBCode extends BCodeSyncAndTry {
286289
cw.toByteArray
287290
}
288291

289-
val Item2(arrivalPos, mirror, plain, bean, outFolder) = item
292+
val Item2(arrivalPos, mirror, plain, bean, _, outFolder) = item
290293

291294
val mirrorC = if (mirror == null) null else SubItem3(mirror.name, getByteArray(mirror))
292295
val plainC = SubItem3(plain.name, getByteArray(plain))

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
9898
* a boolean indicating if the instruction list contains an instantiation of a serializable SAM
9999
* type.
100100
*/
101-
def cloneInstructions(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode]): (InsnList, Map[AbstractInsnNode, AbstractInsnNode], Boolean) = {
101+
def cloneInstructions(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], keepLineNumbers: Boolean): (InsnList, Map[AbstractInsnNode, AbstractInsnNode], Boolean) = {
102102
val javaLabelMap = labelMap.asJava
103103
val result = new InsnList
104104
var map = Map.empty[AbstractInsnNode, AbstractInsnNode]
@@ -112,9 +112,11 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
112112
}
113113
case _ =>
114114
}
115-
val cloned = ins.clone(javaLabelMap)
116-
result add cloned
117-
map += ((ins, cloned))
115+
if (keepLineNumbers || !ins.isInstanceOf[LineNumberNode]) {
116+
val cloned = ins.clone(javaLabelMap)
117+
result add cloned
118+
map += ((ins, cloned))
119+
}
118120
}
119121
(result, map, hasSerializableClosureInstantiation)
120122
}

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

Lines changed: 31 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import scala.tools.asm.Attribute
1515
import scala.tools.nsc.backend.jvm.BackendReporting._
1616
import scala.tools.nsc.util.ClassPath
1717
import BytecodeUtils._
18-
import ByteCodeRepository._
1918
import BTypes.InternalName
2019
import java.util.concurrent.atomic.AtomicLong
2120

@@ -29,9 +28,10 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT)
2928
import btypes._
3029

3130
/**
32-
* ClassNodes for classes being compiled in the current compilation run.
31+
* Contains ClassNodes and the canonical path of the source file path of classes being compiled in
32+
* the current compilation run.
3333
*/
34-
val compilingClasses: concurrent.Map[InternalName, ClassNode] = recordPerRunCache(concurrent.TrieMap.empty)
34+
val compilingClasses: concurrent.Map[InternalName, (ClassNode, String)] = recordPerRunCache(concurrent.TrieMap.empty)
3535

3636
/**
3737
* Cache for parsed ClassNodes.
@@ -67,40 +67,45 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT)
6767
}
6868
}
6969

70-
def add(classNode: ClassNode, source: Source) = {
71-
if (source == CompilationUnit) compilingClasses(classNode.name) = classNode
72-
else parsedClasses(classNode.name) = Right((classNode, lruCounter.incrementAndGet()))
70+
def add(classNode: ClassNode, sourceFilePath: Option[String]) = sourceFilePath match {
71+
case Some(path) if path != "<no file>" => compilingClasses(classNode.name) = (classNode, path)
72+
case _ => parsedClasses(classNode.name) = Right((classNode, lruCounter.incrementAndGet()))
73+
}
74+
75+
private def parsedClassNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = {
76+
val r = parsedClasses.get(internalName) match {
77+
case Some(l @ Left(_)) => l
78+
case Some(r @ Right((classNode, _))) =>
79+
parsedClasses(internalName) = Right((classNode, lruCounter.incrementAndGet()))
80+
r
81+
case None =>
82+
limitCacheSize()
83+
val res = parseClass(internalName).map((_, lruCounter.incrementAndGet()))
84+
parsedClasses(internalName) = res
85+
res
86+
}
87+
r.map(_._1)
7388
}
7489

7590
/**
76-
* The class node and source for an internal name. If the class node is not yet available, it is
77-
* parsed from the classfile on the compile classpath.
91+
* The class node and source file path (if the class is being compiled) for an internal name. If
92+
* the class node is not yet available, it is parsed from the classfile on the compile classpath.
7893
*/
79-
def classNodeAndSource(internalName: InternalName): Either[ClassNotFound, (ClassNode, Source)] = {
80-
classNode(internalName) map (n => {
81-
val source = if (compilingClasses contains internalName) CompilationUnit else Classfile
82-
(n, source)
83-
})
94+
def classNodeAndSourceFilePath(internalName: InternalName): Either[ClassNotFound, (ClassNode, Option[String])] = {
95+
compilingClasses.get(internalName) match {
96+
case Some((c, p)) => Right((c, Some(p)))
97+
case _ => parsedClassNode(internalName).map((_, None))
98+
}
8499
}
85100

86101
/**
87102
* The class node for an internal name. If the class node is not yet available, it is parsed from
88103
* the classfile on the compile classpath.
89104
*/
90105
def classNode(internalName: InternalName): Either[ClassNotFound, ClassNode] = {
91-
compilingClasses.get(internalName).map(Right(_)) getOrElse {
92-
val r = parsedClasses.get(internalName) match {
93-
case Some(l @ Left(_)) => l
94-
case Some(r @ Right((classNode, _))) =>
95-
parsedClasses(internalName) = Right((classNode, lruCounter.incrementAndGet()))
96-
r
97-
case None =>
98-
limitCacheSize()
99-
val res = parseClass(internalName).map((_, lruCounter.incrementAndGet()))
100-
parsedClasses(internalName) = res
101-
res
102-
}
103-
r.map(_._1)
106+
compilingClasses.get(internalName) match {
107+
case Some((c, _)) => Right(c)
108+
case None => parsedClassNode(internalName)
104109
}
105110
}
106111

@@ -289,13 +294,3 @@ class ByteCodeRepository[BT <: BTypes](val classPath: ClassPath, val btypes: BT)
289294
}
290295
}
291296
}
292-
293-
object ByteCodeRepository {
294-
/**
295-
* The source of a ClassNode in the ByteCodeRepository. Can be either [[CompilationUnit]] if the
296-
* class is being compiled or [[Classfile]] if the class was parsed from the compilation classpath.
297-
*/
298-
sealed trait Source
299-
object CompilationUnit extends Source
300-
object Classfile extends Source
301-
}

‎src/compiler/scala/tools/nsc/backend/jvm/opt/CallGraph.scala

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import scala.collection.JavaConverters._
1616
import scala.tools.nsc.backend.jvm.BTypes.InternalName
1717
import scala.tools.nsc.backend.jvm.BackendReporting._
1818
import scala.tools.nsc.backend.jvm.analysis._
19-
import ByteCodeRepository.{Source, CompilationUnit}
2019
import BytecodeUtils._
2120

2221
class CallGraph[BT <: BTypes](val btypes: BT) {
@@ -128,17 +127,17 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
128127
methodNode.instructions.iterator.asScala foreach {
129128
case call: MethodInsnNode if a.frameAt(call) != null => // skips over unreachable code
130129
val callee: Either[OptimizerWarning, Callee] = for {
131-
(method, declarationClass) <- byteCodeRepository.methodNode(call.owner, call.name, call.desc): Either[OptimizerWarning, (MethodNode, InternalName)]
132-
(declarationClassNode, source) <- byteCodeRepository.classNodeAndSource(declarationClass): Either[OptimizerWarning, (ClassNode, Source)]
130+
(method, declarationClass) <- byteCodeRepository.methodNode(call.owner, call.name, call.desc): Either[OptimizerWarning, (MethodNode, InternalName)]
131+
(declarationClassNode, calleeSourceFilePath) <- byteCodeRepository.classNodeAndSourceFilePath(declarationClass): Either[OptimizerWarning, (ClassNode, Option[String])]
133132
} yield {
134133
val declarationClassBType = classBTypeFromClassNode(declarationClassNode)
135-
val info = analyzeCallsite(method, declarationClassBType, call, source)
134+
val info = analyzeCallsite(method, declarationClassBType, call, calleeSourceFilePath)
136135
import info._
137136
Callee(
138137
callee = method,
139138
calleeDeclarationClass = declarationClassBType,
140139
safeToInline = safeToInline,
141-
canInlineFromSource = canInlineFromSource,
140+
sourceFilePath = sourceFilePath,
142141
annotatedInline = annotatedInline,
143142
annotatedNoInline = annotatedNoInline,
144143
samParamTypes = info.samParamTypes,
@@ -256,15 +255,15 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
256255
/**
257256
* Just a named tuple used as return type of `analyzeCallsite`.
258257
*/
259-
private case class CallsiteInfo(safeToInline: Boolean, canInlineFromSource: Boolean,
258+
private case class CallsiteInfo(safeToInline: Boolean, sourceFilePath: Option[String],
260259
annotatedInline: Boolean, annotatedNoInline: Boolean,
261260
samParamTypes: IntMap[ClassBType],
262261
warning: Option[CalleeInfoWarning])
263262

264263
/**
265264
* Analyze a callsite and gather meta-data that can be used for inlining decisions.
266265
*/
267-
private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, call: MethodInsnNode, calleeSource: Source): CallsiteInfo = {
266+
private def analyzeCallsite(calleeMethodNode: MethodNode, calleeDeclarationClassBType: ClassBType, call: MethodInsnNode, calleeSourceFilePath: Option[String]): CallsiteInfo = {
268267
val methodSignature = calleeMethodNode.name + calleeMethodNode.desc
269268

270269
try {
@@ -273,8 +272,6 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
273272
// callee, we only check there for the methodInlineInfo, we should find it there.
274273
calleeDeclarationClassBType.info.orThrow.inlineInfo.methodInfos.get(methodSignature) match {
275274
case Some(methodInlineInfo) =>
276-
val canInlineFromSource = compilerSettings.optInlineGlobal || calleeSource == CompilationUnit
277-
278275
val isAbstract = BytecodeUtils.isAbstractMethod(calleeMethodNode)
279276

280277
val receiverType = classBTypeFromParsedClassfile(call.owner)
@@ -308,26 +305,26 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
308305
// static impl method first (safeToRewrite).
309306
CallsiteInfo(
310307
safeToInline =
311-
canInlineFromSource &&
308+
inlinerHeuristics.canInlineFromSource(calleeSourceFilePath) &&
312309
isStaticallyResolved && // (1)
313310
!isAbstract &&
314311
!BytecodeUtils.isConstructor(calleeMethodNode) &&
315312
!BytecodeUtils.isNativeMethod(calleeMethodNode) &&
316313
!BytecodeUtils.hasCallerSensitiveAnnotation(calleeMethodNode),
317-
canInlineFromSource = canInlineFromSource,
314+
sourceFilePath = calleeSourceFilePath,
318315
annotatedInline = methodInlineInfo.annotatedInline,
319316
annotatedNoInline = methodInlineInfo.annotatedNoInline,
320317
samParamTypes = samParamTypes(calleeMethodNode, receiverType),
321318
warning = warning)
322319

323320
case None =>
324321
val warning = MethodInlineInfoMissing(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, calleeDeclarationClassBType.info.orThrow.inlineInfo.warning)
325-
CallsiteInfo(false, false, false, false, IntMap.empty, Some(warning))
322+
CallsiteInfo(false, None, false, false, IntMap.empty, Some(warning))
326323
}
327324
} catch {
328325
case Invalid(noInfo: NoClassBTypeInfo) =>
329326
val warning = MethodInlineInfoError(calleeDeclarationClassBType.internalName, calleeMethodNode.name, calleeMethodNode.desc, noInfo)
330-
CallsiteInfo(false, false, false, false, IntMap.empty, Some(warning))
327+
CallsiteInfo(false, None, false, false, IntMap.empty, Some(warning))
331328
}
332329
}
333330

@@ -389,7 +386,7 @@ class CallGraph[BT <: BTypes](val btypes: BT) {
389386
* gathering the information about this callee.
390387
*/
391388
final case class Callee(callee: MethodNode, calleeDeclarationClass: btypes.ClassBType,
392-
safeToInline: Boolean, canInlineFromSource: Boolean,
389+
safeToInline: Boolean, sourceFilePath: Option[String],
393390
annotatedInline: Boolean, annotatedNoInline: Boolean,
394391
samParamTypes: IntMap[btypes.ClassBType],
395392
calleeInfoWarning: Option[CalleeInfoWarning]) {

‎src/compiler/scala/tools/nsc/backend/jvm/opt/ClosureOptimizer.scala

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import scala.tools.nsc.backend.jvm.BTypes.InternalName
1717
import BytecodeUtils._
1818
import BackendReporting._
1919
import Opcodes._
20-
import scala.tools.nsc.backend.jvm.opt.ByteCodeRepository.CompilationUnit
2120
import scala.collection.JavaConverters._
2221

2322
class ClosureOptimizer[BT <: BTypes](val btypes: BT) {
@@ -354,16 +353,15 @@ class ClosureOptimizer[BT <: BTypes](val btypes: BT) {
354353

355354
// the method node is needed for building the call graph entry
356355
val bodyMethod = byteCodeRepository.methodNode(lambdaBodyHandle.getOwner, lambdaBodyHandle.getName, lambdaBodyHandle.getDesc)
357-
def bodyMethodIsBeingCompiled = byteCodeRepository.classNodeAndSource(lambdaBodyHandle.getOwner).map(_._2 == CompilationUnit).getOrElse(false)
356+
val sourceFilePath = byteCodeRepository.compilingClasses.get(lambdaBodyHandle.getOwner).map(_._2)
358357
val callee = bodyMethod.map({
359358
case (bodyMethodNode, bodyMethodDeclClass) =>
360359
val bodyDeclClassType = classBTypeFromParsedClassfile(bodyMethodDeclClass)
361-
val canInlineFromSource = compilerSettings.optInlineGlobal || bodyMethodIsBeingCompiled
362360
Callee(
363361
callee = bodyMethodNode,
364362
calleeDeclarationClass = bodyDeclClassType,
365-
safeToInline = canInlineFromSource,
366-
canInlineFromSource = canInlineFromSource,
363+
safeToInline = inlinerHeuristics.canInlineFromSource(sourceFilePath),
364+
sourceFilePath = sourceFilePath,
367365
annotatedInline = false,
368366
annotatedNoInline = false,
369367
samParamTypes = callGraph.samParamTypes(bodyMethodNode, bodyDeclClassType),

‎src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
253253
def inlineCallsite(callsite: Callsite): Unit = {
254254
import callsite.{callsiteClass, callsiteMethod, callsiteInstruction, receiverKnownNotNull, callsiteStackHeight}
255255
val Right(callsiteCallee) = callsite.callee
256-
import callsiteCallee.{callee, calleeDeclarationClass}
256+
import callsiteCallee.{callee, calleeDeclarationClass, sourceFilePath}
257257

258258
// Inlining requires the callee not to have unreachable code, the analyzer used below should not
259259
// return any `null` frames. Note that inlining a method can create unreachable code. Example:
@@ -268,11 +268,14 @@ class Inliner[BT <: BTypes](val btypes: BT) {
268268

269269
// New labels for the cloned instructions
270270
val labelsMap = cloneLabels(callee)
271-
val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap)
272-
val keepLineNumbers = callsiteClass == calleeDeclarationClass
273-
if (!keepLineNumbers) {
274-
removeLineNumberNodes(clonedInstructions)
271+
val sameSourceFile = sourceFilePath match {
272+
case Some(calleeSource) => byteCodeRepository.compilingClasses.get(callsiteClass.internalName) match {
273+
case Some((_, `calleeSource`)) => true
274+
case _ => false
275+
}
276+
case _ => false
275277
}
278+
val (clonedInstructions, instructionMap, hasSerializableClosureInstantiation) = cloneInstructions(callee, labelsMap, keepLineNumbers = sameSourceFile)
276279

277280
// local vars in the callee are shifted by the number of locals at the callsite
278281
val localVarShift = callsiteMethod.maxLocals

0 commit comments

Comments
 (0)