Skip to content

Commit e9a9a43

Browse files
melezovnicolasstucki
melezov
authored and
nicolasstucki
committed
SI-9571 Avoid boxing primitives in string concatenation
Port of cb68d9c
1 parent 078a866 commit e9a9a43

File tree

4 files changed

+120
-46
lines changed

4 files changed

+120
-46
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,9 +1088,17 @@ trait BCodeBodyBuilder extends BCodeSkelBuilder {
10881088
case concatenations =>
10891089
bc.genStartConcat
10901090
for (elem <- concatenations) {
1091-
val kind = tpeTK(elem)
1092-
genLoad(elem, kind)
1093-
bc.genStringConcat(kind)
1091+
val loadedElem = elem match {
1092+
case Apply(boxOp, value :: Nil) if isBox(boxOp.symbol) =>
1093+
// Eliminate boxing of primitive values. Boxing is introduced by erasure because
1094+
// there's only a single synthetic `+` method "added" to the string class.
1095+
value
1096+
1097+
case _ => elem
1098+
}
1099+
val elemType = tpeTK(loadedElem)
1100+
genLoad(loadedElem, elemType)
1101+
bc.genConcat(elemType)
10941102
}
10951103
bc.genEndConcat
10961104

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -222,16 +222,19 @@ trait BCodeIdiomatic {
222222
/*
223223
* can-multi-thread
224224
*/
225-
final def genStringConcat(el: BType): Unit = {
226-
val jtype = el match {
225+
def genConcat(elemType: BType): Unit = {
226+
val paramType = elemType match {
227227
case ct: ClassBType if ct.isSubtypeOf(StringRef) => StringRef
228228
case ct: ClassBType if ct.isSubtypeOf(jlStringBufferRef) => jlStringBufferRef
229229
case ct: ClassBType if ct.isSubtypeOf(jlCharSequenceRef) => jlCharSequenceRef
230-
case rt: RefBType => ObjectReference
231-
case pt: PrimitiveBType => pt // Currently this ends up being boxed in erasure
230+
// Don't match for `ArrayBType(CHAR)`, even though StringBuilder has such an overload:
231+
// `"a" + Array('b')` should NOT be "ab", but "a[C@...".
232+
case _: RefBType => ObjectReference
233+
// jlStringBuilder does not have overloads for byte and short, but we can just use the int version
234+
case BYTE | SHORT => INT
235+
case pt: PrimitiveBType => pt
232236
}
233-
234-
val bt = MethodBType(List(jtype), jlStringBuilderRef)
237+
val bt = MethodBType(List(paramType), jlStringBuilderRef)
235238
invokevirtual(JavaStringBuilderClassName, "append", bt.descriptor)
236239
}
237240

‎test/files/specialized/fft.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
Processing 65536 items
22
Boxed doubles: 0
3-
Boxed ints: 2
3+
Boxed ints: 0
44
Boxed longs: 1179811

‎test/junit/scala/tools/nsc/backend/jvm/StringConcatTest.scala

Lines changed: 99 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,47 +24,110 @@ class StringConcatTest extends ClearAfterClass {
2424
ClearAfterClass.stateToClear = StringConcatTest
2525
val compiler = StringConcatTest.compiler
2626

27-
val commonPreInstructions = List(Label(0), LineNumber(1, Label(0)), TypeOp(NEW, "java/lang/StringBuilder"), Op(DUP), Invoke(INVOKESPECIAL, "java/lang/StringBuilder", "<init>", "()V", false), Ldc(LDC, "abc"), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/String;)Ljava/lang/StringBuilder;", false), VarOp(ALOAD, 0))
28-
29-
val commonPostInstructions = List(Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "toString", "()Ljava/lang/String;", false), Op(ARETURN), Label(12))
30-
31-
def instructionsWithCommonParts(instructions: List[Instruction]) = commonPreInstructions ++ instructions ++ commonPostInstructions
32-
33-
def instructionsForResultMethod(code: String): List[Instruction] = {
34-
val methods = compileMethods(compiler)(code)
35-
val resultMethod = methods.find(_.name == "result").get
36-
instructionsFromMethod(resultMethod)
37-
}
38-
3927
@Test
40-
def concatStringToStringBuilder: Unit = {
41-
val code = """ def string = "def"; def result = "abc" + string """
42-
val actualInstructions = instructionsForResultMethod(code)
43-
val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "string", "()Ljava/lang/String;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/String;)Ljava/lang/StringBuilder;", false)))
44-
assertSameCode(actualInstructions, expectedInstructions)
45-
}
28+
def appendOverloadNoBoxing(): Unit = {
29+
val code =
30+
"""class C {
31+
| def t1(
32+
| v: Unit,
33+
| z: Boolean,
34+
| c: Char,
35+
| b: Byte,
36+
| s: Short,
37+
| i: Int,
38+
| l: Long,
39+
| f: Float,
40+
| d: Double,
41+
| str: String,
42+
| sbuf: java.lang.StringBuffer,
43+
| chsq: java.lang.CharSequence,
44+
| chrs: Array[Char]) = str + this + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
45+
|
46+
| // similar, but starting off with any2stringadd
47+
| def t2(
48+
| v: Unit,
49+
| z: Boolean,
50+
| c: Char,
51+
| b: Byte,
52+
| s: Short,
53+
| i: Int,
54+
| l: Long,
55+
| f: Float,
56+
| d: Double,
57+
| str: String,
58+
| sbuf: java.lang.StringBuffer,
59+
| chsq: java.lang.CharSequence,
60+
| chrs: Array[Char]) = this + str + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
61+
|}
62+
""".stripMargin
63+
val List(c) = compileClasses(compiler)(code)
4664

47-
@Test
48-
def concatStringBufferToStringBuilder: Unit = {
49-
val code = """ def stringBuffer = new java.lang.StringBuffer("def"); def result = "abc" + stringBuffer """
50-
val actualInstructions = instructionsForResultMethod(code)
51-
val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "stringBuffer", "()Ljava/lang/StringBuffer;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;", false)))
52-
assertSameCode(actualInstructions, expectedInstructions)
53-
}
65+
def invokeNameDesc(m: String): List[String] = getSingleMethod(c, m).instructions collect {
66+
case Invoke(_, _, name, desc, _) => name + desc
67+
}
68+
assertEquals(invokeNameDesc("t1"), List(
69+
"<init>()V",
70+
"append(Ljava/lang/String;)Ljava/lang/StringBuilder;",
71+
"append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
72+
"append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
73+
"append(Z)Ljava/lang/StringBuilder;",
74+
"append(C)Ljava/lang/StringBuilder;",
75+
"append(I)Ljava/lang/StringBuilder;",
76+
"append(I)Ljava/lang/StringBuilder;",
77+
"append(I)Ljava/lang/StringBuilder;",
78+
"append(F)Ljava/lang/StringBuilder;",
79+
"append(J)Ljava/lang/StringBuilder;",
80+
"append(D)Ljava/lang/StringBuilder;",
81+
"append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;",
82+
"append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;",
83+
"append(Ljava/lang/Object;)Ljava/lang/StringBuilder;", // test that we're not using the [C overload
84+
"toString()Ljava/lang/String;"))
5485

55-
@Test
56-
def concatCharSequenceToStringBuilder: Unit = {
57-
val code = """ def charSequence: CharSequence = "def"; def result = "abc" + charSequence """
58-
val actualInstructions = instructionsForResultMethod(code)
59-
val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "charSequence", "()Ljava/lang/CharSequence;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;", false)))
60-
assertSameCode(actualInstructions, expectedInstructions)
86+
assertEquals(invokeNameDesc("t2"), List(
87+
"<init>()V",
88+
"any2stringadd(Ljava/lang/Object;)Ljava/lang/Object;",
89+
"$plus$extension(Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/String;",
90+
"append(Ljava/lang/String;)Ljava/lang/StringBuilder;",
91+
"append(Ljava/lang/Object;)Ljava/lang/StringBuilder;",
92+
"append(Z)Ljava/lang/StringBuilder;",
93+
"append(C)Ljava/lang/StringBuilder;",
94+
"append(I)Ljava/lang/StringBuilder;",
95+
"append(I)Ljava/lang/StringBuilder;",
96+
"append(I)Ljava/lang/StringBuilder;",
97+
"append(F)Ljava/lang/StringBuilder;",
98+
"append(J)Ljava/lang/StringBuilder;",
99+
"append(D)Ljava/lang/StringBuilder;",
100+
"append(Ljava/lang/StringBuffer;)Ljava/lang/StringBuilder;",
101+
"append(Ljava/lang/CharSequence;)Ljava/lang/StringBuilder;",
102+
"append(Ljava/lang/Object;)Ljava/lang/StringBuilder;", // test that we're not using the [C overload
103+
"toString()Ljava/lang/String;"))
61104
}
62105

63106
@Test
64-
def concatIntToStringBuilder: Unit = {
65-
val code = """ def int = 123; def result = "abc" + int """
66-
val actualInstructions = instructionsForResultMethod(code)
67-
val expectedInstructions = instructionsWithCommonParts(List(Invoke(INVOKEVIRTUAL, "C", "int", "()I", false), Invoke(INVOKESTATIC, "scala/runtime/BoxesRunTime", "boxToInteger", "(I)Ljava/lang/Integer;", false), Invoke(INVOKEVIRTUAL, "java/lang/StringBuilder", "append", "(Ljava/lang/Object;)Ljava/lang/StringBuilder;", false)))
68-
assertSameCode(actualInstructions, expectedInstructions)
107+
def concatPrimitiveCorrectness(): Unit = {
108+
val obj: Object = new { override def toString = "TTT" }
109+
def t(
110+
v: Unit,
111+
z: Boolean,
112+
c: Char,
113+
b: Byte,
114+
s: Short,
115+
i: Int,
116+
l: Long,
117+
f: Float,
118+
d: Double,
119+
str: String,
120+
sbuf: java.lang.StringBuffer,
121+
chsq: java.lang.CharSequence,
122+
chrs: Array[Char]) = {
123+
val s1 = str + obj + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
124+
val s2 = obj + str + v + z + c + b + s + i + f + l + d + sbuf + chsq + chrs
125+
s1 + "//" + s2
126+
}
127+
def sbuf = { val r = new java.lang.StringBuffer(); r.append("sbuf"); r }
128+
def chsq: java.lang.CharSequence = "chsq"
129+
val s = t((), true, 'd', 3: Byte, 12: Short, 3, -32l, 12.3f, -4.2d, "me", sbuf, chsq, Array('a', 'b'))
130+
val r = s.replaceAll("""\[C@\w+""", "<ARRAY>")
131+
assertEquals(r, "meTTT()trued312312.3-32-4.2sbufchsq<ARRAY>//TTTme()trued312312.3-32-4.2sbufchsq<ARRAY>")
69132
}
70133
}

0 commit comments

Comments
 (0)