Skip to content

Commit 153e6ef

Browse files
lihaoyilihaoyi
committed
RefChecks: reuse member-type scratch sets (est. 0.07% speedup)
Concrete-class member type checking now reuses RefChecks-owned Name and Symbol scratch sets instead of allocating fresh util.HashSet tables for every class. The path is hot because baseline allocation profiling showed 2.22 MiB of object arrays under RefChecks.checkMemberTypesOK, and replacing per-class table allocation with occupied-slot clearing cuts the repeated setup work visible in Arrays.fill. The scratch sets keep the same hash/equality behavior, bound retained capacity, and use a busy guard with nested fallbacks so stale entries and reentrant checkAllOverrides frames preserve the previous behavior. Expected changes: - Arrays.fill self% and tot% should improve: reused member-type sets clear only occupied slots instead of repeatedly allocating and clearing fresh hash-table arrays. - MegaPhase.transformTree tot% should improve: RefChecks runs inside the broad phase traversal, so less member-type set setup should reduce the enclosing transform total. - HashSet.isEqual self% could regress: global equality/hash-set traffic can move slightly as the old util.HashSet membership checks disappear from this path and other hash-set work becomes more visible. - Symbol.isTerm self% could regress: the member declaration scan still tests every candidate symbol, so this unchanged predicate can move up when nearby set-management time is removed. - No other regressions expected: equality/hash behavior matches util.HashSet, occupied slots are nulled after every class, reentrant calls get separate scratch sets, and unusually large tables are reset above the retained-capacity bound. JFR profile deltas (5 repeats × 10 runs, mean ± stddev, iter-7/run-0 → iter-7/run-15): 1. Arrays.fill self%: 0.26 ± 0.05 → 0.16 ± 0.03 (-0.10, 2.0σ) 2. Arrays.fill tot%: 0.26 ± 0.05 → 0.17 ± 0.03 (-0.09, 1.8σ) 3. MegaPhase.transformTree tot%: 15.19 ± 0.13 → 14.07 ± 0.85 (-1.12, 1.3σ) 4. HashSet.isEqual self%: 0.19 ± 0.01 → 0.22 ± 0.02 (+0.03, 1.5σ) 5. Symbol.isTerm self%: 0.36 ± 0.04 → 0.40 ± 0.04 (+0.04, 1.0σ) Estimated total speedup: 0.07 ± 0.06 (from rows 1 and 4 above; rows 2 and 3 are overlapping confirmation, row 5 is a scan guardrail) Accepted. Arrays.fill self-time improves by 2.0σ and total time by 1.8σ, while the enclosing MegaPhase.transformTree total moves down. HashSet.isEqual has a small global self regression and Symbol.isTerm is at threshold, but the removed allocation path plus net direct self-time win make the scratch reuse worth carrying.
1 parent 31596c8 commit 153e6ef

1 file changed

Lines changed: 162 additions & 41 deletions

File tree

‎compiler/src/dotty/tools/dotc/typer/RefChecks.scala‎

Lines changed: 162 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,122 @@ object RefChecks {
3232
val name: String = "refchecks"
3333
val description: String = "checks related to abstract members and overriding"
3434

35+
final class MemberTypeScratch:
36+
private val membersToCheck = new ScratchSet[Name](4096, maxRetainedCapacity = 16384)
37+
private val seenClasses = new ScratchSet[Symbol](256, maxRetainedCapacity = 1024)
38+
private var inUse = false
39+
40+
def withSets[T](op: (ScratchSet[Name], ScratchSet[Symbol]) => T): T =
41+
if inUse then
42+
val nestedMembersToCheck = new ScratchSet[Name](4096, maxRetainedCapacity = 16384)
43+
val nestedSeenClasses = new ScratchSet[Symbol](256, maxRetainedCapacity = 1024)
44+
try op(nestedMembersToCheck, nestedSeenClasses)
45+
finally
46+
nestedMembersToCheck.clear()
47+
nestedSeenClasses.clear()
48+
else
49+
inUse = true
50+
try op(membersToCheck, seenClasses)
51+
finally
52+
membersToCheck.clear()
53+
seenClasses.clear()
54+
inUse = false
55+
56+
final class ScratchSet[T <: AnyRef](initialCapacity: Int, maxRetainedCapacity: Int):
57+
private val initialTableCapacity = roundToPower(initialCapacity)
58+
private val initialSlotCapacity = 16 min initialTableCapacity
59+
private var table = new Array[AnyRef | Null](initialTableCapacity)
60+
private var slots = new Array[Int](initialSlotCapacity)
61+
private var used = 0
62+
private var limit = tableLimit(table.length)
63+
64+
private def tableLimit(capacity: Int): Int = capacity / 2
65+
66+
private def roundToPower(n: Int): Int =
67+
if n < 4 then 4
68+
else 1 << (32 - Integer.numberOfLeadingZeros(n - 1))
69+
70+
private def hash(key: T): Int =
71+
val h = key.hashCode
72+
val i = (h ^ (h >>> 16)) * 0x85EBCA6B
73+
val j = (i ^ (i >>> 13)) & 0x7FFFFFFF
74+
if j == 0 then 0x41081989 else j
75+
76+
private def index(hash: Int): Int = hash & (table.length - 1)
77+
78+
private def ensureSlotCapacity(capacity: Int): Unit =
79+
if capacity > slots.length then
80+
val slots1 = new Array[Int](slots.length * 2)
81+
Array.copy(slots, 0, slots1, 0, slots.length)
82+
slots = slots1
83+
84+
private def addFresh(elem: T): Unit =
85+
var idx = index(hash(elem))
86+
while table(idx) != null do
87+
idx = index(idx + 1)
88+
table(idx) = elem
89+
ensureSlotCapacity(used + 1)
90+
slots(used) = idx
91+
used += 1
92+
93+
private def growTable(): Unit =
94+
val oldTable = table
95+
val oldSlots = slots
96+
val oldUsed = used
97+
table = new Array[AnyRef | Null](oldTable.length * 2)
98+
slots = new Array[Int](oldSlots.length max oldUsed)
99+
used = 0
100+
limit = tableLimit(table.length)
101+
var i = 0
102+
while i < oldUsed do
103+
addFresh(oldTable(oldSlots(i)).asInstanceOf[T])
104+
i += 1
105+
106+
private def resetTable(): Unit =
107+
table = new Array[AnyRef | Null](initialTableCapacity)
108+
slots = new Array[Int](initialSlotCapacity)
109+
used = 0
110+
limit = tableLimit(table.length)
111+
112+
def clear(): Unit =
113+
if table.length > maxRetainedCapacity then resetTable()
114+
else
115+
var i = 0
116+
while i < used do
117+
table(slots(i)) = null
118+
i += 1
119+
used = 0
120+
121+
def add(elem: T): Boolean =
122+
var idx = index(hash(elem))
123+
var entry = table(idx)
124+
while entry != null do
125+
if entry.equals(elem) then return false
126+
idx = index(idx + 1)
127+
entry = table(idx)
128+
table(idx) = elem
129+
ensureSlotCapacity(used + 1)
130+
slots(used) = idx
131+
used += 1
132+
if used > limit then growTable()
133+
true
134+
135+
def contains(elem: T): Boolean =
136+
var idx = index(hash(elem))
137+
var entry = table(idx)
138+
while entry != null do
139+
if entry.equals(elem) then return true
140+
idx = index(idx + 1)
141+
entry = table(idx)
142+
false
143+
144+
def foreach[U](f: T => U): Unit =
145+
var idx = 0
146+
while idx < table.length do
147+
val elem = table(idx)
148+
if elem != null then f(elem.asInstanceOf[T])
149+
idx += 1
150+
35151
private val defaultMethodFilter = new NameFilter {
36152
def apply(pre: Type, name: Name)(using Context): Boolean = name.is(DefaultGetterName)
37153
def isStable = true
@@ -321,7 +437,10 @@ object RefChecks {
321437
* @param makeOverridingPairsChecker A function for creating a OverridePairsChecker instance
322438
* from the class symbol and the self type
323439
*/
324-
def checkAllOverrides(clazz: ClassSymbol, makeOverridingPairsChecker: ((ClassSymbol, Type) => Context ?=> OverridingPairsChecker) | Null = null)(using Context): Unit = {
440+
def checkAllOverrides(
441+
clazz: ClassSymbol,
442+
makeOverridingPairsChecker: ((ClassSymbol, Type) => Context ?=> OverridingPairsChecker) | Null = null,
443+
memberTypeScratch: MemberTypeScratch | Null = null)(using Context): Unit = {
325444
val self = clazz.thisType
326445
val upwardsSelf = upwardsThisType(clazz)
327446
var hasErrors = false
@@ -884,46 +1003,46 @@ object RefChecks {
8841003
// the rules of overriding and linearization. This method checks that the implementation has indeed
8851004
// a type that subsumes the full member type.
8861005
def checkMemberTypesOK() = {
887-
888-
// First compute all member names we need to check in `membersToCheck`.
889-
// We do not check
890-
// - types
891-
// - synthetic members or bridges
892-
// - members in other concrete classes, since these have been checked before
893-
// (this is done for efficiency)
894-
// - members in a prefix of inherited parents that all come from Java or Scala2
895-
// (this is done to avoid false positives since Scala2's rules for checking are different)
896-
val membersToCheck = new util.HashSet[Name](4096)
897-
val seenClasses = new util.HashSet[Symbol](256)
898-
def addDecls(cls: Symbol): Unit =
899-
if (!seenClasses.contains(cls)) {
900-
seenClasses += cls
901-
for (mbr <- cls.info.decls)
902-
if (mbr.isTerm && !mbr.isOneOf(Synthetic | Bridge) && mbr.memberCanMatchInheritedSymbols &&
903-
!membersToCheck.contains(mbr.name))
904-
membersToCheck += mbr.name
905-
cls.info.parents.map(_.classSymbol)
906-
.filter(_.isOneOf(AbstractOrTrait))
907-
.dropWhile(_.isOneOf(JavaDefined | Scala2x))
908-
.foreach(addDecls)
909-
}
910-
addDecls(clazz)
911-
912-
// For each member, check that the type of its symbol, as seen from `self`
913-
// can override the info of this member
914-
withMode(Mode.IgnoreCaptures) {
915-
for (name <- membersToCheck)
916-
for (mbrd <- self.member(name).alternatives) {
917-
val mbr = mbrd.symbol
918-
val mbrType = mbr.info.asSeenFrom(self, mbr.owner)
919-
if (!mbrType.overrides(mbrd.info, matchLoosely = true))
920-
report.errorOrMigrationWarning(
921-
em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz
922-
| its type $mbrType
923-
| does not conform to ${mbrd.info}""",
924-
(if (mbr.owner == clazz) mbr else clazz).srcPos, MigrationVersion.Scala2to3)
1006+
def checkWithScratch(membersToCheck: ScratchSet[Name], seenClasses: ScratchSet[Symbol]) =
1007+
// First compute all member names we need to check in `membersToCheck`.
1008+
// We do not check
1009+
// - types
1010+
// - synthetic members or bridges
1011+
// - members in other concrete classes, since these have been checked before
1012+
// (this is done for efficiency)
1013+
// - members in a prefix of inherited parents that all come from Java or Scala2
1014+
// (this is done to avoid false positives since Scala2's rules for checking are different)
1015+
def addDecls(cls: Symbol): Unit =
1016+
if (seenClasses.add(cls)) {
1017+
for (mbr <- cls.info.decls)
1018+
if (mbr.isTerm && !mbr.isOneOf(Synthetic | Bridge) && mbr.memberCanMatchInheritedSymbols)
1019+
membersToCheck.add(mbr.name)
1020+
cls.info.parents.map(_.classSymbol)
1021+
.filter(_.isOneOf(AbstractOrTrait))
1022+
.dropWhile(_.isOneOf(JavaDefined | Scala2x))
1023+
.foreach(addDecls)
1024+
}
1025+
addDecls(clazz)
1026+
1027+
// For each member, check that the type of its symbol, as seen from `self`
1028+
// can override the info of this member
1029+
withMode(Mode.IgnoreCaptures) {
1030+
for (name <- membersToCheck)
1031+
for (mbrd <- self.member(name).alternatives) {
1032+
val mbr = mbrd.symbol
1033+
val mbrType = mbr.info.asSeenFrom(self, mbr.owner)
1034+
if (!mbrType.overrides(mbrd.info, matchLoosely = true))
1035+
report.errorOrMigrationWarning(
1036+
em"""${mbr.showLocated} is not a legal implementation of `$name` in $clazz
1037+
| its type $mbrType
1038+
| does not conform to ${mbrd.info}""",
1039+
(if (mbr.owner == clazz) mbr else clazz).srcPos, MigrationVersion.Scala2to3)
1040+
}
9251041
}
926-
}
1042+
if memberTypeScratch == null then
1043+
val scratch = new MemberTypeScratch
1044+
scratch.withSets(checkWithScratch)
1045+
else memberTypeScratch.withSets(checkWithScratch)
9271046
}
9281047

9291048
/** Check that inheriting a case class does not constitute a variant refinement
@@ -1479,6 +1598,8 @@ class RefChecks extends MiniPhase { thisPhase =>
14791598

14801599
import tpd.*
14811600

1601+
private val memberTypeScratch = new MemberTypeScratch
1602+
14821603
override def phaseName: String = RefChecks.name
14831604

14841605
override def description: String = RefChecks.description
@@ -1525,7 +1646,7 @@ class RefChecks extends MiniPhase { thisPhase =>
15251646
checkParents(cls, tree.parents)
15261647
if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _))
15271648
checkCompanionNameClashes(cls)
1528-
checkAllOverrides(cls)
1649+
checkAllOverrides(cls, memberTypeScratch = memberTypeScratch)
15291650
checkImplicitNotFoundAnnotation.template(cls.classDenot)
15301651
tree
15311652
} catch {

0 commit comments

Comments
 (0)