Skip to content

Commit 873a69a

Browse files
committed
Performance optimization: Precompute SELECTin criteria
Computing whether we need a SELECTin seems to be expensive and the test is wrong 99.99% of the time. As an optimization, we mark the trees that need a SELECTin at the time we first detect the problem.
1 parent 87092af commit 873a69a

File tree

3 files changed

+32
-11
lines changed

3 files changed

+32
-11
lines changed

compiler/src/dotty/tools/dotc/config/Config.scala

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,6 @@ object Config {
8787
*/
8888
final val alignArgsInAnd = true
8989

90-
/** If this flag is set, higher-kinded applications are checked for validity
91-
*/
92-
final val checkHKApplications = false
93-
9490
/** If this flag is set, method types are checked for valid parameter references
9591
*/
9692
final val checkMethodTypes = false
@@ -140,6 +136,14 @@ object Config {
140136
*/
141137
final val checkAtomsComparisons = false
142138

139+
/** Normally, a Select node can be unpickled giving just the name and the signature
140+
* of the selected member. However, in some rare cases, this is not enough since there
141+
* are multiple possible members with the same signature. In that case we need to
142+
* pickle the Select with a SELECTin tag. If on, we check that all ambiguous selects do
143+
* get pickled with SELECTin
144+
*/
145+
final val checkAmbiguousSelects = false
146+
143147
/** In `derivedSelect`, rewrite
144148
*
145149
* (S & T)#A --> S#A & T#A

compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import StdNames.nme
1616
import transform.SymUtils._
1717
import printing.Printer
1818
import printing.Texts._
19-
import util.SourceFile
19+
import util.{SourceFile, Property}
20+
import config.Config.checkAmbiguousSelects
2021
import annotation.constructorOnly
2122

2223
object TreePickler {
@@ -30,6 +31,11 @@ object TreePickler {
3031
if isTermHole then s"{{{ $idx |" ~~ printer.toTextGlobal(tpe) ~~ "|" ~~ printer.toTextGlobal(args, ", ") ~~ "}}}"
3132
else s"[[[ $idx |" ~~ printer.toTextGlobal(tpe) ~~ "|" ~~ printer.toTextGlobal(args, ", ") ~~ "]]]"
3233
}
34+
35+
/** Select tree cannot be unambiguously resolved with signature alone,
36+
* needs to be pickled using the SELECTin tag.
37+
*/
38+
val NeedsSelectIn = new Property.StickyKey[Unit]
3339
}
3440

3541
class TreePickler(pickler: TastyPickler) {
@@ -383,12 +389,14 @@ class TreePickler(pickler: TastyPickler) {
383389
}
384390
case _ =>
385391
val sig = tree.tpe.signature
386-
val isAmbiguous =
387-
sig != Signature.NotAMethod
388-
&& qual.tpe.nonPrivateMember(name).match
392+
val needsSelectIn = tree.hasAttachment(NeedsSelectIn)
393+
if checkAmbiguousSelects then
394+
val isAmbiguous = qual.tpe.nonPrivateMember(name) match
389395
case d: MultiDenotation => d.atSignature(sig).isInstanceOf[MultiDenotation]
390396
case _ => false
391-
if isAmbiguous then
397+
assert(needsSelectIn == isAmbiguous,
398+
i"ambiguity misprediction for select node $tree, is ambiguous = $isAmbiguous")
399+
if needsSelectIn then
392400
writeByte(SELECTin)
393401
withLength {
394402
pickleNameAndSig(name, tree.symbol.signature)

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import rewrites.Rewrites.patch
4040
import NavigateAST._
4141
import transform.SymUtils._
4242
import transform.TypeUtils._
43+
import tasty.TreePickler.NeedsSelectIn
4344
import reporting.trace
4445
import Nullables.{NotNullInfo, given _}
4546
import NullOpsDecorator._
@@ -2827,9 +2828,16 @@ class Typer extends Namer
28272828
typr.println(i"adapt overloaded $ref with alternatives ${altDenots map (_.info)}%\n\n %")
28282829
def altRef(alt: SingleDenotation) = TermRef(ref.prefix, ref.name, alt)
28292830
val alts = altDenots.map(altRef)
2831+
2832+
def resolveWith(alt: TermRef) =
2833+
val resolved = tree.withType(alt)
2834+
if alts.exists(a => (a ne alt) && a.signature == alt.signature) then
2835+
resolved.pushAttachment(NeedsSelectIn, ())
2836+
readaptSimplified(resolved)
2837+
28302838
resolveOverloaded(alts, pt) match {
28312839
case alt :: Nil =>
2832-
readaptSimplified(tree.withType(alt))
2840+
resolveWith(alt)
28332841
case Nil =>
28342842
// If alternative matches, there are still two ways to recover:
28352843
// 1. If context is an application, try to insert an apply or implicit
@@ -2844,7 +2852,8 @@ class Typer extends Namer
28442852
tryInsertApplyOrImplicit(tree, pt, locked)(noMatches)
28452853
case _ =>
28462854
alts.filter(_.info.isParameterless) match {
2847-
case alt :: Nil => readaptSimplified(tree.withType(alt))
2855+
case alt :: Nil =>
2856+
resolveWith(alt)
28482857
case _ =>
28492858
if (altDenots exists (_.info.paramInfoss == ListOfNil))
28502859
typed(untpd.Apply(untpd.TypedSplice(tree), Nil), pt, locked)

0 commit comments

Comments
 (0)