Skip to content

Commit 763c2a5

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. in runtime reflection can undergo is init. #3 is dangerous and needs protection
1 parent bbcd419 commit 763c2a5

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.
@@ -3104,8 +3111,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
31043111
override def thisType: Type = {
31053112
val period = thisTypePeriod
31063113
if (period != currentPeriod) {
3107-
thisTypePeriod = currentPeriod
31083114
if (!isValid(period)) thisTypeCache = ThisType(this)
3115+
thisTypePeriod = currentPeriod
31093116
}
31103117
thisTypeCache
31113118
}
@@ -3193,9 +3200,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
31933200
override def typeOfThis = {
31943201
val period = typeOfThisPeriod
31953202
if (period != currentPeriod) {
3196-
typeOfThisPeriod = currentPeriod
31973203
if (!isValid(period))
31983204
typeOfThisCache = singleType(owner.thisType, sourceModule)
3205+
typeOfThisPeriod = currentPeriod
31993206
}
32003207
typeOfThisCache
32013208
}
@@ -3206,9 +3213,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
32063213
// Skip a package object class, because the members are also in
32073214
// the package and we wish to avoid spurious ambiguities as in pos/t3999.
32083215
if (!isPackageObjectClass) {
3216+
implicitMembersCacheValue = tp.implicitMembers
32093217
implicitMembersCacheKey1 = tp
32103218
implicitMembersCacheKey2 = tp.decls.elems
3211-
implicitMembersCacheValue = tp.implicitMembers
32123219
}
32133220
}
32143221
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)