Skip to content

Commit cc6a3c9

Browse files
retronymretronym
committed
SI-7475 Pay the duplication tax for findMember/findMembers
Swathes of important logic are duplicated between `findMember` and `findMembers` after this run of optimization. d905558 Variation #10 to optimze findMember fcb0c01 Attempt #9 to opimize findMember. 71d2ceb Attempt #8 to opimize findMember. 77e5692 Attempty #7 to optimize findMember 275115e Fixing problem that caused fingerprints to fail in reflection. Also fixed test case that failed when moving to findMember e94252e Attemmpt #6 to optimize findMember 73e61b8 Attempt #5 to optimize findMember. 04f0b65 Attempt #4 to optimize findMember 0e3c70f Attempt #3 to optimize findMember 41f4497 Attempt #2 to optimize findMember 1a73aa0 Attempt #1 to optimize findMember This commit updates `findMembers` with the bug fixes that `findMember` has received, and adds flashing warning signs for future maintainers. A followup commit will address the duplication at the root.
1 parent c1da44c commit cc6a3c9

File tree

1 file changed

+65
-23
lines changed

1 file changed

+65
-23
lines changed

‎src/reflect/scala/reflect/internal/Types.scala

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,9 @@ trait Types
985985
else (baseClasses.head.newOverloaded(this, alts))
986986
}
987987

988+
// !!!
989+
// !!! DUPLICATED LOGIC ALERT, edits in `findMembers` here must be replicated in `findMember`.
990+
// !!!
988991
def findMembers(excludedFlags: Long, requiredFlags: Long): Scope = {
989992
def findMembersInternal: Scope = {
990993
var members: Scope = null
@@ -996,6 +999,8 @@ trait Types
996999
var excluded = excludedFlags | DEFERRED
9971000
var continue = true
9981001
var self: Type = null
1002+
var seenFirstNonRefinementClass: Boolean = false
1003+
var refinementParents: List[Symbol] = Nil
9991004
while (continue) {
10001005
continue = false
10011006
val bcs0 = baseClasses
@@ -1008,34 +1013,53 @@ trait Types
10081013
val flags = sym.flags
10091014
if ((flags & required) == required) {
10101015
val excl = flags & excluded
1011-
if (excl == 0L &&
1012-
(// omit PRIVATE LOCALS unless selector class is contained in class owning the def.
1013-
(bcs eq bcs0) ||
1014-
(flags & PrivateLocal) != PrivateLocal ||
1015-
(bcs0.head.hasTransOwner(bcs.head)))) {
1016+
val isMember = (
1017+
excl == 0L
1018+
&& (
1019+
(flags & PRIVATE) != PRIVATE // non-privates are always members
1020+
|| (
1021+
!seenFirstNonRefinementClass // classes don't inherit privates
1022+
|| refinementParents.contains(bcs.head) // refinements inherit privates of direct parents
1023+
)
1024+
)
1025+
)
1026+
1027+
// !!!
1028+
// !!! DUPLICATED LOGIC ALERT, edits in `findMembers` here must be replicated in `findMember`.
1029+
// !!!
1030+
if (isMember) {
10161031
if (members eq null) members = newFindMemberScope
10171032
var others: ScopeEntry = members.lookupEntry(sym.name)
10181033
var symtpe: Type = null
1019-
while ((others ne null) && {
1020-
val other = others.sym
1021-
(other ne sym) &&
1022-
((other.owner eq sym.owner) ||
1023-
(flags & PRIVATE) != 0 || {
1024-
if (self eq null) self = narrowForFindMember(this)
1025-
if (symtpe eq null) symtpe = self.memberType(sym)
1026-
!(self.memberType(other) matches symtpe)
1027-
})}) {
1028-
others = members lookupNextEntry others
1034+
@inline def computeSelfAndSymtpe() = {
1035+
if (self eq null) self = narrowForFindMember(this)
1036+
if (symtpe eq null) symtpe = self.memberType(sym)
10291037
}
1038+
while ( (others ne null)
1039+
&& {
1040+
val other = others.sym
1041+
( (other ne sym)
1042+
&& ( (other.owner eq sym.owner)
1043+
|| (flags & PRIVATE) != 0
1044+
|| { computeSelfAndSymtpe(); !(self.memberType(other) matches symtpe)}
1045+
)
1046+
)
1047+
}
1048+
) others = members lookupNextEntry others
10301049
if (others eq null) members enter sym
10311050
} else if (excl == DEFERRED) {
10321051
continue = true
10331052
}
10341053
}
10351054
entry = entry.next
10361055
} // while (entry ne null)
1037-
// excluded = excluded | LOCAL
1038-
bcs = bcs.tail
1056+
val sym = bcs.head
1057+
if (sym.isRefinementClass)
1058+
refinementParents :::= bcs.head.parentSymbols // keep track of direct parents of refinements
1059+
else if (sym.isClass)
1060+
seenFirstNonRefinementClass = true
1061+
1062+
bcs = bcs.tail // Unlike `findMember`, we can't when constructor here.
10391063
} // while (!bcs.isEmpty)
10401064
required |= DEFERRED
10411065
excluded &= ~(DEFERRED.toLong)
@@ -1052,12 +1076,15 @@ trait Types
10521076
* Find member(s) in this type. If several members matching criteria are found, they are
10531077
* returned in an OverloadedSymbol
10541078
*
1055-
* @param name The member's name, where nme.ANYNAME means `unspecified`
1079+
* @param name The member's name
10561080
* @param excludedFlags Returned members do not have these flags
10571081
* @param requiredFlags Returned members do have these flags
10581082
* @param stableOnly If set, return only members that are types or stable values
10591083
*/
10601084
//TODO: use narrow only for modules? (correct? efficiency gain?)
1085+
// !!!
1086+
// !!! DUPLICATED LOGIC ALERT, edits in `findMember` must be replicated in `findMembers`.
1087+
// !!!
10611088
def findMember(name: Name, excludedFlags: Long, requiredFlags: Long, stableOnly: Boolean): Symbol = {
10621089
def findMemberInternal: Symbol = {
10631090
var member: Symbol = NoSymbol
@@ -1070,9 +1097,9 @@ trait Types
10701097
var membertpe: Type = null
10711098
var required = requiredFlags
10721099
var excluded: Long = excludedFlags | DEFERRED
1073-
var seenFirstNonRefinementClass: Boolean = false
10741100
var continue = true
10751101
var self: Type = null
1102+
var seenFirstNonRefinementClass: Boolean = false
10761103
var refinementParents: List[Symbol] = Nil
10771104

10781105
while (continue) {
@@ -1097,6 +1124,10 @@ trait Types
10971124
)
10981125
)
10991126
)
1127+
1128+
// !!!
1129+
// !!! DUPLICATED LOGIC ALERT, edits in `findMember` must be replicated in `findMembers`.
1130+
// !!!
11001131
if (isMember) {
11011132
if (name.isTypeName || (stableOnly && sym.isStable && !sym.hasVolatileType)) {
11021133
if (Statistics.canEnable) Statistics.popTimer(typeOpsStack, start)
@@ -1122,12 +1153,20 @@ trait Types
11221153
} else { // Already found 2 or more members
11231154
var others: List[Symbol] = members
11241155
var symtpe: Type = null
1125-
@inline def computeSelfAndSymtpe() = {
1126-
if (self eq null) self = narrowForFindMember(this)
1127-
if (symtpe eq null) symtpe = self.memberType(sym)
1156+
@inline
1157+
def isNewMember(other: Symbol): Boolean = {
1158+
( (other ne sym)
1159+
&& ( (other.owner eq sym.owner)
1160+
|| (flags & PRIVATE) != 0
1161+
|| {
1162+
if (self eq null) { self = narrowForFindMember(this); symtpe = self.memberType(sym) }
1163+
!(self.memberType(other) matches symtpe)
1164+
}
1165+
)
1166+
)
11281167
}
11291168
while ( (others ne null)
1130-
&& {
1169+
&& isNewMember(others.head)
11311170
val other = others.head
11321171
( (other ne sym)
11331172
&& ( (other.owner eq sym.owner)
@@ -1150,6 +1189,9 @@ trait Types
11501189
entry = decls lookupNextEntry entry
11511190
} // while (entry ne null)
11521191

1192+
// !!!
1193+
// !!! DUPLICATED LOGIC ALERT, edits in `findMember` must be replicated in `findMembers`.
1194+
// !!!
11531195
val sym = bcs.head
11541196
if (sym.isRefinementClass)
11551197
refinementParents :::= bcs.head.parentSymbols // keep track of direct parents of refinements

0 commit comments

Comments
 (0)