Skip to content
This repository was archived by the owner on Sep 1, 2020. It is now read-only.

Commit 961c8be

Browse files
authored
Merge pull request scala#5290 from lrytz/sd48
SD-48 limit the lenght of inlined local variable names
2 parents 4ba2584 + 2b1e4ef commit 961c8be

File tree

4 files changed

+68
-10
lines changed

4 files changed

+68
-10
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,4 +1164,7 @@ object BTypes {
11641164
// no static way (without symbol table instance) to get to nme.ScalaATTR / ScalaSignatureATTR
11651165
val ScalaAttributeName = "Scala"
11661166
val ScalaSigAttributeName = "ScalaSig"
1167+
1168+
// when inlining, local variable names of the callee are prefixed with the name of the callee method
1169+
val InlinedLocalVariablePrefixMaxLenght = 128
11671170
}

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,33 @@ object BytecodeUtils {
324324
* Clone the local variable descriptors of `methodNode` and map their `start` and `end` labels
325325
* according to the `labelMap`.
326326
*/
327-
def cloneLocalVariableNodes(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], prefix: String, shift: Int): List[LocalVariableNode] = {
328-
methodNode.localVariables.iterator().asScala.map(localVariable => new LocalVariableNode(
329-
prefix + localVariable.name,
330-
localVariable.desc,
331-
localVariable.signature,
332-
labelMap(localVariable.start),
333-
labelMap(localVariable.end),
334-
localVariable.index + shift
335-
)).toList
327+
def cloneLocalVariableNodes(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], calleeMethodName: String, shift: Int): List[LocalVariableNode] = {
328+
methodNode.localVariables.iterator().asScala.map(localVariable => {
329+
val name =
330+
if (calleeMethodName.length + localVariable.name.length < BTypes.InlinedLocalVariablePrefixMaxLenght) {
331+
calleeMethodName + "_" + localVariable.name
332+
} else {
333+
val parts = localVariable.name.split("_").toVector
334+
val (methNames, varName) = (calleeMethodName +: parts.init, parts.last)
335+
// keep at least 5 characters per method name
336+
val maxNumMethNames = BTypes.InlinedLocalVariablePrefixMaxLenght / 5
337+
val usedMethNames =
338+
if (methNames.length < maxNumMethNames) methNames
339+
else {
340+
val half = maxNumMethNames / 2
341+
methNames.take(half) ++ methNames.takeRight(half)
342+
}
343+
val charsPerMethod = BTypes.InlinedLocalVariablePrefixMaxLenght / usedMethNames.length
344+
usedMethNames.foldLeft("")((res, methName) => res + methName.take(charsPerMethod) + "_") + varName
345+
}
346+
new LocalVariableNode(
347+
name,
348+
localVariable.desc,
349+
localVariable.signature,
350+
labelMap(localVariable.start),
351+
labelMap(localVariable.end),
352+
localVariable.index + shift)
353+
}).toList
336354
}
337355

338356
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
382382
callsiteMethod.instructions.insert(callsiteInstruction, clonedInstructions)
383383
callsiteMethod.instructions.remove(callsiteInstruction)
384384

385-
callsiteMethod.localVariables.addAll(cloneLocalVariableNodes(callee, labelsMap, callee.name + "_", localVarShift).asJava)
385+
callsiteMethod.localVariables.addAll(cloneLocalVariableNodes(callee, labelsMap, callee.name, localVarShift).asJava)
386386
// prepend the handlers of the callee. the order of handlers matters: when an exception is thrown
387387
// at some instruction, the first handler guarding that instruction and having a matching exception
388388
// type is executed. prepending the callee's handlers makes sure to test those handlers first if

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,4 +1587,41 @@ class InlinerTest extends BytecodeTesting {
15871587
val List(c, t) = compile(code)
15881588
assertNoIndy(getMethod(c, "t1"))
15891589
}
1590+
1591+
@Test
1592+
def limitInlinedLocalVariableNames(): Unit = {
1593+
val code =
1594+
"""class C {
1595+
| def f(x: Int): Int = x
1596+
| @inline final def methodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1597+
| f(param)
1598+
| @inline final def anotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1599+
| methodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1600+
| @inline final def oneMoreMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1601+
| anotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1602+
| @inline final def yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1603+
| oneMoreMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1604+
| @inline final def oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1605+
| yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1606+
| def t(p: Int) =
1607+
| oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(p)) +
1608+
| oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(p))
1609+
|}
1610+
""".stripMargin
1611+
1612+
val List(c) = compile(code)
1613+
assertEquals(getAsmMethod(c, "t").localVariables.asScala.toList.map(l => (l.name, l.index)).sortBy(_._2),List(
1614+
("this",0),
1615+
("p",1),
1616+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence_param",2),
1617+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchS_yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFren_param",3),
1618+
("oneLastMethodWithVeryVeryLongNameAlmostLik_yetAnotherMethodWithVeryVeryLongNameAlmost_oneMoreMethodWithVeryVeryLongNameAlmostLik_param",4),
1619+
("oneLastMethodWithVeryVeryLongNam_yetAnotherMethodWithVeryVeryLong_oneMoreMethodWithVeryVeryLongNam_anotherMethodWithVeryVeryLongNam_param",5),
1620+
("oneLastMethodWithVeryVery_yetAnotherMethodWithVeryV_oneMoreMethodWithVeryVery_anotherMethodWithVeryVery_methodWithVeryVeryLongNam_param",6),
1621+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence_param",7),
1622+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchS_yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFren_param",8),
1623+
("oneLastMethodWithVeryVeryLongNameAlmostLik_yetAnotherMethodWithVeryVeryLongNameAlmost_oneMoreMethodWithVeryVeryLongNameAlmostLik_param",9),
1624+
("oneLastMethodWithVeryVeryLongNam_yetAnotherMethodWithVeryVeryLong_oneMoreMethodWithVeryVeryLongNam_anotherMethodWithVeryVeryLongNam_param",10),
1625+
("oneLastMethodWithVeryVery_yetAnotherMethodWithVeryV_oneMoreMethodWithVeryVery_anotherMethodWithVeryVery_methodWithVeryVeryLongNam_param",11)))
1626+
}
15901627
}

0 commit comments

Comments
 (0)