Skip to content

Commit 6f35f4a

Browse files
authored
Fix #23224: Optimize simple tuple extraction (#23373)
Close #23224: This PR optimizes simple tuple extraction by avoiding unnecessary tuple allocations and refines the typing of bind patterns for named tuples. * Optimise `makePatDef` to reduce tuple creation when a pattern uses only simple variables or wildcards. * If the selector of a match has bottom type, use the type from pattern for the bind variable. For example: ```scala def f1: (Int, Int, Int) = (1, 2, 3) def test1 = val (a, b, c) = f1 a + b + c ``` Before this PR: ```scala val $1$: (Int, Int, Int) = this.f1:(Int, Int, Int) @unchecked match { case Tuple3.unapply[Int, Int, Int](a @ _, b @ _, c @ _) => Tuple3.apply[Int, Int, Int](a, b, c) } val a: Int = $1$._1 val b: Int = $1$._2 val c: Int = $1$._3 a + b + c ``` After this PR: ```scala val $2$: (Int, Int, Int) = this.f1:(Int, Int, Int) @unchecked match { case $1$ @ Tuple3.unapply[Int, Int, Int](_, _, _) => $1$:(Int, Int, Int) } val a: Int = $2$._1 val b: Int = $2$._2 val c: Int = $2$._3 a + b + c ``` Also in genBCode now: ```scala val $2$: Tuple3 = matchResult1[Tuple3]: { case val x1: Tuple3 = this.f1():Tuple3 if x1 ne null then { case val $1$: Tuple3 = x1 return[matchResult1] $1$:Tuple3 } else () throw new MatchError(x1) } val a: Int = Int.unbox($2$._1()) val b: Int = Int.unbox($2$._2()) val c: Int = Int.unbox($2$._3()) a + b + c ``` I use the regular expression (`val\s*\(\s*[a-zA-Z_]\w*(\s*,\s*[a-zA-Z_]\w*)*\s*\)\s*=`) to search in the compiler, and found 400+ places which are simple tuple extraction like this.
2 parents 01447df + 82c068c commit 6f35f4a

File tree

8 files changed

+163
-32
lines changed

8 files changed

+163
-32
lines changed

compiler/src/dotty/tools/dotc/ast/Desugar.scala

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,6 +1450,27 @@ object desugar {
14501450
sel
14511451
end match
14521452

1453+
case class TuplePatternInfo(arity: Int, varNum: Int, wildcardNum: Int)
1454+
object TuplePatternInfo:
1455+
def apply(pat: Tree)(using Context): TuplePatternInfo = pat match
1456+
case Tuple(pats) =>
1457+
var arity = 0
1458+
var varNum = 0
1459+
var wildcardNum = 0
1460+
pats.foreach: p =>
1461+
arity += 1
1462+
p match
1463+
case id: Ident if !isBackquoted(id) =>
1464+
if id.name.isVarPattern then
1465+
varNum += 1
1466+
if id.name == nme.WILDCARD then
1467+
wildcardNum += 1
1468+
case _ =>
1469+
TuplePatternInfo(arity, varNum, wildcardNum)
1470+
case _ =>
1471+
TuplePatternInfo(-1, -1, -1)
1472+
end TuplePatternInfo
1473+
14531474
/** If `pat` is a variable pattern,
14541475
*
14551476
* val/var/lazy val p = e
@@ -1483,30 +1504,47 @@ object desugar {
14831504
|please bind to an identifier and use an alias given.""", bind)
14841505
false
14851506

1486-
def isTuplePattern(arity: Int): Boolean = pat match {
1487-
case Tuple(pats) if pats.size == arity =>
1488-
pats.forall(isVarPattern)
1489-
case _ => false
1490-
}
1491-
1492-
val isMatchingTuple: Tree => Boolean = {
1493-
case Tuple(es) => isTuplePattern(es.length) && !hasNamedArg(es)
1494-
case _ => false
1495-
}
1507+
val tuplePatternInfo = TuplePatternInfo(pat)
1508+
1509+
// When desugaring a PatDef in general, we use pattern matching on the rhs
1510+
// and collect the variable values in a tuple, then outside the match,
1511+
// we destructure the tuple to get the individual variables.
1512+
// We can achieve two kinds of tuple optimizations if the pattern is a tuple
1513+
// of simple variables or wildcards:
1514+
// 1. Full optimization:
1515+
// If the rhs is known to produce a literal tuple of the same arity,
1516+
// we can directly fetch the values from the tuple.
1517+
// For example: `val (x, y) = if ... then (1, "a") else (2, "b")` becomes
1518+
// `val $1$ = if ...; val x = $1$._1; val y = $1$._2`.
1519+
// 2. Partial optimization:
1520+
// If the rhs can be typed as a tuple and matched with correct arity, we can
1521+
// return the tuple itself in the case if there are no more than one variable
1522+
// in the pattern, or return the the value if there is only one variable.
1523+
1524+
val fullTupleOptimizable =
1525+
val isMatchingTuple: Tree => Boolean = {
1526+
case Tuple(es) => tuplePatternInfo.varNum == es.length && !hasNamedArg(es)
1527+
case _ => false
1528+
}
1529+
tuplePatternInfo.arity > 0
1530+
&& tuplePatternInfo.arity == tuplePatternInfo.varNum
1531+
&& forallResults(rhs, isMatchingTuple)
14961532

1497-
// We can only optimize `val pat = if (...) e1 else e2` if:
1498-
// - `e1` and `e2` are both tuples of arity N
1499-
// - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`
1500-
val tupleOptimizable = forallResults(rhs, isMatchingTuple)
1533+
val partialTupleOptimizable =
1534+
tuplePatternInfo.arity > 0
1535+
&& tuplePatternInfo.arity == tuplePatternInfo.varNum
1536+
// We exclude the case where there is only one variable,
1537+
// because it should be handled by `makeTuple` directly.
1538+
&& tuplePatternInfo.wildcardNum < tuplePatternInfo.arity - 1
15011539

15021540
val inAliasGenerator = original match
15031541
case _: GenAlias => true
15041542
case _ => false
15051543

1506-
val vars =
1507-
if (tupleOptimizable) // include `_`
1544+
val vars: List[VarInfo] =
1545+
if fullTupleOptimizable || partialTupleOptimizable then // include `_`
15081546
pat match
1509-
case Tuple(pats) => pats.map { case id: Ident => id -> TypeTree() }
1547+
case Tuple(pats) => pats.map { case id: Ident => (id, TypeTree()) }
15101548
else
15111549
getVariables(
15121550
tree = pat,
@@ -1517,12 +1555,27 @@ object desugar {
15171555
errorOnGivenBinding
15181556
) // no `_`
15191557

1520-
val ids = for ((named, _) <- vars) yield Ident(named.name)
1558+
val ids = for ((named, tpt) <- vars) yield Ident(named.name)
1559+
15211560
val matchExpr =
1522-
if (tupleOptimizable) rhs
1561+
if fullTupleOptimizable then rhs
15231562
else
1524-
val caseDef = CaseDef(pat, EmptyTree, makeTuple(ids).withAttachment(ForArtifact, ()))
1563+
val caseDef =
1564+
if partialTupleOptimizable then
1565+
val tmpTuple = UniqueName.fresh()
1566+
// Replace all variables with wildcards in the pattern
1567+
val pat1 = pat match
1568+
case Tuple(pats) =>
1569+
val wildcardPats = pats.map(p => Ident(nme.WILDCARD).withSpan(p.span))
1570+
Tuple(wildcardPats).withSpan(pat.span)
1571+
CaseDef(
1572+
Bind(tmpTuple, pat1),
1573+
EmptyTree,
1574+
Ident(tmpTuple).withAttachment(ForArtifact, ())
1575+
)
1576+
else CaseDef(pat, EmptyTree, makeTuple(ids).withAttachment(ForArtifact, ()))
15251577
Match(makeSelector(rhs, MatchCheck.IrrefutablePatDef), caseDef :: Nil)
1578+
15261579
vars match {
15271580
case Nil if !mods.is(Lazy) =>
15281581
matchExpr

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,14 +350,16 @@ trait TreeInfo[T <: Untyped] { self: Trees.Instance[T] =>
350350
}
351351

352352
/** Checks whether predicate `p` is true for all result parts of this expression,
353-
* where we zoom into Ifs, Matches, and Blocks.
353+
* where we zoom into Ifs, Matches, Tries, and Blocks.
354354
*/
355-
def forallResults(tree: Tree, p: Tree => Boolean): Boolean = tree match {
355+
def forallResults(tree: Tree, p: Tree => Boolean): Boolean = tree match
356356
case If(_, thenp, elsep) => forallResults(thenp, p) && forallResults(elsep, p)
357-
case Match(_, cases) => cases forall (c => forallResults(c.body, p))
357+
case Match(_, cases) => cases.forall(c => forallResults(c.body, p))
358+
case Try(_, cases, finalizer) =>
359+
cases.forall(c => forallResults(c.body, p))
360+
&& (finalizer.isEmpty || forallResults(finalizer, p))
358361
case Block(_, expr) => forallResults(expr, p)
359362
case _ => p(tree)
360-
}
361363

362364
/** The tree stripped of the possibly nested applications (term and type).
363365
* The original tree if it's not an application.

compiler/src/dotty/tools/dotc/typer/Applications.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1721,7 +1721,8 @@ trait Applications extends Compatibility {
17211721
if selType <:< unapplyArgType then
17221722
unapp.println(i"case 1 $unapplyArgType ${ctx.typerState.constraint}")
17231723
fullyDefinedType(unapplyArgType, "pattern selector", tree.srcPos)
1724-
selType.dropAnnot(defn.UncheckedAnnot) // need to drop @unchecked. Just because the selector is @unchecked, the pattern isn't.
1724+
if selType.isBottomType then unapplyArgType
1725+
else selType.dropAnnot(defn.UncheckedAnnot) // need to drop @unchecked. Just because the selector is @unchecked, the pattern isn't.
17251726
else
17261727
if !ctx.mode.is(Mode.InTypeTest) then
17271728
checkMatchable(selType, tree.srcPos, pattern = true)
@@ -1743,7 +1744,7 @@ trait Applications extends Compatibility {
17431744
val unapplyPatterns = UnapplyArgs(unapplyApp.tpe, unapplyFn, unadaptedArgs, tree.srcPos)
17441745
.typedPatterns(qual, this)
17451746
val result = assignType(cpy.UnApply(tree)(newUnapplyFn, unapplyImplicits(dummyArg, unapplyApp), unapplyPatterns), ownType)
1746-
if (ownType.stripped eq selType.stripped) || ownType.isError then result
1747+
if (ownType.stripped eq selType.stripped) || selType.isBottomType || ownType.isError then result
17471748
else tryWithTypeTest(Typed(result, TypeTree(ownType)), selType)
17481749
case tp =>
17491750
val unapplyErr = if (tp.isError) unapplyFn else notAnExtractor(unapplyFn)

compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2827,6 +2827,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
28272827
if isStableIdentifierOrLiteral || isNamedTuplePattern then pt
28282828
else if isWildcardStarArg(body1)
28292829
|| pt == defn.ImplicitScrutineeTypeRef
2830+
|| pt.isBottomType
28302831
|| body1.tpe <:< pt // There is some strange interaction with gadt matching.
28312832
// and implicit scopes.
28322833
// run/t2755.scala fails to compile if this subtype test is omitted

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ trait DottyBytecodeTest {
133133
}, l.stringLines)
134134
}
135135

136+
def assertNoInvoke(m: MethodNode, receiver: String, method: String): Unit =
137+
assertNoInvoke(instructionsFromMethod(m), receiver, method)
138+
def assertNoInvoke(l: List[Instruction], receiver: String, method: String): Unit = {
139+
assert(!l.exists {
140+
case Invoke(_, `receiver`, `method`, _, _) => true
141+
case _ => false
142+
}, s"Found unexpected invoke of $receiver.$method in:\n${l.stringLines}")
143+
}
144+
136145
def diffInstructions(isa: List[Instruction], isb: List[Instruction]): String = {
137146
val len = Math.max(isa.length, isb.length)
138147
val sb = new StringBuilder

compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,6 +1606,28 @@ class DottyBytecodeTests extends DottyBytecodeTest {
16061606
}
16071607
}
16081608

1609+
@Test
1610+
def simpleTupleExtraction(): Unit = {
1611+
val code =
1612+
"""class C {
1613+
| def f1(t: (Int, String)) =
1614+
| val (i, s) = t
1615+
| i + s.length
1616+
|}
1617+
""".stripMargin
1618+
checkBCode(code) { dir =>
1619+
val c = loadClassNode(dir.lookupName("C.class", directory = false).input)
1620+
val f1 = getMethod(c, "f1")
1621+
assertNoInvoke(f1, "scala/Tuple2$", "apply") // no Tuple2.apply call
1622+
// no `new` instruction
1623+
val hasNew = instructionsFromMethod(f1).exists {
1624+
case Op(Opcodes.NEW) => true
1625+
case _ => false
1626+
}
1627+
assertFalse("f1 should not have NEW instruction", hasNew)
1628+
}
1629+
}
1630+
16091631
@Test
16101632
def deprecation(): Unit = {
16111633
val code =

tests/neg/i7294.check

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
-- [E007] Type Mismatch Error: tests/neg/i7294.scala:7:15 --------------------------------------------------------------
1+
-- [E007] Type Mismatch Error: tests/neg/i7294.scala:7:18 --------------------------------------------------------------
22
7 | case x: T => x.g(10) // error
3-
| ^
4-
| Found: (x : Nothing)
5-
| Required: ?{ g: ? }
6-
| Note that implicit conversions were not tried because the result of an implicit conversion
7-
| must be more specific than ?{ g: [applied to (10) returning T] }
3+
| ^^^^^^^
4+
| Found: Any
5+
| Required: T
6+
|
7+
| where: T is a type in given instance f with bounds <: foo.Foo
88
|
99
| longer explanation available when compiling with `-explain`

tests/pos/simple-tuple-extract.scala

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
2+
class Test:
3+
def f1: (Int, String, AnyRef) = (1, "2", "3")
4+
def f2: (x: Int, y: String) = (0, "y")
5+
6+
def test1 =
7+
val (a, b, c) = f1
8+
// Desugared to:
9+
// val $2$: (Int, String, AnyRef) =
10+
// this.f1:(Int, String, AnyRef) @unchecked match
11+
// {
12+
// case $1$ @ Tuple3.unapply[Int, String, Object](_, _, _) =>
13+
// $1$:(Int, String, AnyRef)
14+
// }
15+
// val a: Int = $2$._1
16+
// val b: String = $2$._2
17+
// val c: AnyRef = $2$._3
18+
a + b.length() + c.toString.length()
19+
20+
// This pattern will not be optimized:
21+
// val (a1, b1, c1: String) = f1
22+
23+
def test2 =
24+
val (_, b, c) = f1
25+
b.length() + c.toString.length()
26+
27+
val (a2, _, c2) = f1
28+
a2 + c2.toString.length()
29+
30+
val (a3, _, _) = f1
31+
a3 + 1
32+
33+
def test3 =
34+
val (_, b, _) = f1
35+
b.length() + 1
36+
37+
def test4 =
38+
val (x, y) = f2
39+
x + y.length()
40+
41+
def test5 =
42+
val (_, b) = f2
43+
b.length() + 1

0 commit comments

Comments
 (0)