Skip to content

Commit 9666731

Browse files
committed
optimizes Scala reflection GIL
First of all, GIL should only apply to runtime reflection, because noone is going to run toolboxes in multiple threads: a) that's impossible, b/c the compiler isn't thread safe, b) ToolBox api prevents that. Secondly, the only things in symbols which require synchronization are: 1) info/validTo (completers aren't thread-safe), 2) rawInfo and its dependencies (it shares a mutable field with info) 3) non-trivial caches like in typeAsMemberOfLock If you think about it, other things like sourceModule or associatedFile don't need synchronization, because they are either set up when a symbol is created or cloned or when it's completed. The former is obviously safe, while the latter is safe as well, because before acquiring init-dependent state of symbols, the compiler calls `initialize`, which is synchronized. We can say that symbols can be in four possible states: 1) being created, 2) created, but not yet initialized, 3) initializing, 4) initialized. Of those only #3 is dangerous and needs protection, which is what this commit does.
1 parent 85f3bce commit 9666731

File tree

4 files changed

+41
-65
lines changed

4 files changed

+41
-65
lines changed

src/reflect/scala/reflect/internal/Symbols.scala

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -949,6 +949,13 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
949949
final def isInitialized: Boolean =
950950
validTo != NoPeriod
951951

952+
/** Some completers call sym.setInfo when still in-flight and then proceed with initialization (e.g. see LazyPackageType)
953+
* setInfo sets _validTo to current period, which means that after a call to setInfo isInitialized will start returning true.
954+
* Unfortunately, this doesn't mean that info becomes ready to be used, because subsequent initialization might change the info.
955+
* Therefore we need this method to distinguish between initialized and really initialized symbol states.
956+
*/
957+
final def isFullyInitialized: Boolean = _validTo != NoPeriod && (flags & LOCKED) == 0
958+
952959
/** Can this symbol be loaded by a reflective mirror?
953960
*
954961
* Scalac relies on `ScalaSignature' annotation to retain symbols across compilation runs.
@@ -3108,8 +3115,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
31083115
override def thisType: Type = {
31093116
val period = thisTypePeriod
31103117
if (period != currentPeriod) {
3111-
thisTypePeriod = currentPeriod
31123118
if (!isValid(period)) thisTypeCache = ThisType(this)
3119+
thisTypePeriod = currentPeriod
31133120
}
31143121
thisTypeCache
31153122
}
@@ -3197,9 +3204,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
31973204
override def typeOfThis = {
31983205
val period = typeOfThisPeriod
31993206
if (period != currentPeriod) {
3200-
typeOfThisPeriod = currentPeriod
32013207
if (!isValid(period))
32023208
typeOfThisCache = singleType(owner.thisType, sourceModule)
3209+
typeOfThisPeriod = currentPeriod
32033210
}
32043211
typeOfThisCache
32053212
}
@@ -3210,9 +3217,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
32103217
// Skip a package object class, because the members are also in
32113218
// the package and we wish to avoid spurious ambiguities as in pos/t3999.
32123219
if (!isPackageObjectClass) {
3220+
implicitMembersCacheValue = tp.implicitMembers
32133221
implicitMembersCacheKey1 = tp
32143222
implicitMembersCacheKey2 = tp.decls.elems
3215-
implicitMembersCacheValue = tp.implicitMembers
32163223
}
32173224
}
32183225
implicitMembersCacheValue

src/reflect/scala/reflect/runtime/Gil.scala

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@ private[reflect] trait Gil {
1212
lazy val gil = new java.util.concurrent.locks.ReentrantLock
1313

1414
@inline final def gilSynchronized[T](body: => T): T = {
15-
try {
16-
gil.lock()
17-
body
18-
} finally {
19-
gil.unlock()
15+
if (isCompilerUniverse) body
16+
else {
17+
try {
18+
gil.lock()
19+
body
20+
} finally {
21+
gil.unlock()
22+
}
2023
}
2124
}
2225
}

src/reflect/scala/reflect/runtime/SynchronizedOps.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable
5050
// we can keep this lock fine-grained, because methods of Scope don't do anything extraordinary, which makes deadlocks impossible
5151
// fancy subclasses of internal.Scopes#Scope should do synchronization themselves (e.g. see PackageScope for an example)
5252
private lazy val syncLock = new Object
53-
def syncLockSynchronized[T](body: => T): T = syncLock.synchronized { body }
53+
def syncLockSynchronized[T](body: => T): T = if (isCompilerUniverse) body else syncLock.synchronized { body }
5454
override def isEmpty: Boolean = syncLockSynchronized { super.isEmpty }
5555
override def size: Int = syncLockSynchronized { super.size }
5656
override def enter[T <: Symbol](sym: T): T = syncLockSynchronized { super.enter(sym) }

src/reflect/scala/reflect/runtime/SynchronizedSymbols.scala

Lines changed: 22 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,16 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb
2929

3030
trait SynchronizedSymbol extends Symbol {
3131

32-
override def rawflags = gilSynchronized { super.rawflags }
33-
override def rawflags_=(x: Long) = gilSynchronized { super.rawflags_=(x) }
34-
35-
override def rawowner = gilSynchronized { super.rawowner }
36-
override def owner_=(owner: Symbol) = gilSynchronized { super.owner_=(owner) }
37-
38-
override def validTo = gilSynchronized { super.validTo }
39-
override def validTo_=(x: Period) = gilSynchronized { super.validTo_=(x) }
40-
41-
override def pos = gilSynchronized { super.pos }
42-
override def setPos(pos: Position): this.type = { gilSynchronized { super.setPos(pos) }; this }
43-
44-
override def privateWithin = gilSynchronized { super.privateWithin }
45-
override def privateWithin_=(sym: Symbol) = gilSynchronized { super.privateWithin_=(sym) }
32+
def gilSynchronizedIfNotInited[T](body: => T): T = {
33+
if (isFullyInitialized) body
34+
else gilSynchronized { body }
35+
}
4636

47-
override def info = gilSynchronized { super.info }
48-
override def info_=(info: Type) = gilSynchronized { super.info_=(info) }
49-
override def updateInfo(info: Type): Symbol = gilSynchronized { super.updateInfo(info) }
50-
override def rawInfo: Type = gilSynchronized { super.rawInfo }
37+
override def validTo = gilSynchronizedIfNotInited { super.validTo }
38+
override def info = gilSynchronizedIfNotInited { super.info }
39+
override def rawInfo: Type = gilSynchronizedIfNotInited { super.rawInfo }
5140

52-
override def typeParams: List[Symbol] = gilSynchronized {
41+
override def typeParams: List[Symbol] = gilSynchronizedIfNotInited {
5342
if (isCompilerUniverse) super.typeParams
5443
else {
5544
if (isMonomorphicType) Nil
@@ -65,22 +54,14 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb
6554
}
6655
}
6756
}
68-
override def unsafeTypeParams: List[Symbol] = gilSynchronized {
57+
override def unsafeTypeParams: List[Symbol] = gilSynchronizedIfNotInited {
6958
if (isCompilerUniverse) super.unsafeTypeParams
7059
else {
7160
if (isMonomorphicType) Nil
7261
else rawInfo.typeParams
7362
}
7463
}
7564

76-
override def reset(completer: Type): this.type = gilSynchronized { super.reset(completer) }
77-
78-
override def infosString: String = gilSynchronized { super.infosString }
79-
80-
override def annotations: List[AnnotationInfo] = gilSynchronized { super.annotations }
81-
override def setAnnotations(annots: List[AnnotationInfo]): this.type = { gilSynchronized { super.setAnnotations(annots) }; this }
82-
83-
8465
// ------ creators -------------------------------------------------------------------
8566

8667
override protected def createAbstractTypeSymbol(name: TypeName, pos: Position, newFlags: Long): AbstractTypeSymbol =
@@ -128,41 +109,26 @@ private[reflect] trait SynchronizedSymbols extends internal.Symbols { self: Symb
128109

129110
// ------- subclasses ---------------------------------------------------------------------
130111

131-
trait SynchronizedTermSymbol extends TermSymbol with SynchronizedSymbol {
132-
override def name_=(x: Name) = gilSynchronized { super.name_=(x) }
133-
override def rawname = gilSynchronized { super.rawname }
134-
override def referenced: Symbol = gilSynchronized { super.referenced }
135-
override def referenced_=(x: Symbol) = gilSynchronized { super.referenced_=(x) }
136-
}
112+
trait SynchronizedTermSymbol extends SynchronizedSymbol
137113

138114
trait SynchronizedMethodSymbol extends MethodSymbol with SynchronizedTermSymbol {
139-
override def typeAsMemberOf(pre: Type): Type = gilSynchronized { super.typeAsMemberOf(pre) }
140-
override def paramss: List[List[Symbol]] = gilSynchronized { super.paramss }
141-
override def returnType: Type = gilSynchronized { super.returnType }
115+
// we can keep this lock fine-grained, because it's just a cache over asSeenFrom, which makes deadlocks impossible
116+
// unfortunately we cannot elide this lock, because the cache depends on `pre`
117+
private lazy val typeAsMemberOfLock = new Object
118+
override def typeAsMemberOf(pre: Type): Type = gilSynchronizedIfNotInited { typeAsMemberOfLock.synchronized { super.typeAsMemberOf(pre) } }
142119
}
143120

121+
trait SynchronizedModuleSymbol extends ModuleSymbol with SynchronizedTermSymbol
122+
144123
trait SynchronizedTypeSymbol extends TypeSymbol with SynchronizedSymbol {
145-
override def name_=(x: Name) = gilSynchronized { super.name_=(x) }
146-
override def rawname = gilSynchronized { super.rawname }
147-
override def typeConstructor: Type = gilSynchronized { super.typeConstructor }
148-
override def tpe_* : Type = gilSynchronized { super.tpe_* }
149-
override def tpeHK : Type = gilSynchronized { super.tpeHK }
124+
// unlike with typeConstructor, a lock is necessary here, because tpe calculation relies on
125+
// temporarily assigning NoType to tpeCache to detect cyclic reference errors
126+
private lazy val tpeLock = new Object
127+
override def tpe_* : Type = gilSynchronizedIfNotInited { tpeLock.synchronized { super.tpe_* } }
150128
}
151129

152-
trait SynchronizedClassSymbol extends ClassSymbol with SynchronizedTypeSymbol {
153-
override def associatedFile = gilSynchronized { super.associatedFile }
154-
override def associatedFile_=(f: AbstractFile) = gilSynchronized { super.associatedFile_=(f) }
155-
override def thisSym: Symbol = gilSynchronized { super.thisSym }
156-
override def thisType: Type = gilSynchronized { super.thisType }
157-
override def typeOfThis: Type = gilSynchronized { super.typeOfThis }
158-
override def typeOfThis_=(tp: Type) = gilSynchronized { super.typeOfThis_=(tp) }
159-
override def children = gilSynchronized { super.children }
160-
override def addChild(sym: Symbol) = gilSynchronized { super.addChild(sym) }
161-
}
130+
trait SynchronizedClassSymbol extends ClassSymbol with SynchronizedTypeSymbol
162131

163-
trait SynchronizedModuleClassSymbol extends ModuleClassSymbol with SynchronizedClassSymbol {
164-
override def sourceModule = gilSynchronized { super.sourceModule }
165-
override def implicitMembers: Scope = gilSynchronized { super.implicitMembers }
166-
}
132+
trait SynchronizedModuleClassSymbol extends ModuleClassSymbol with SynchronizedClassSymbol
167133
}
168134

0 commit comments

Comments
 (0)