Skip to content

Commit d05566c

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 484d6d7 commit d05566c

File tree

5 files changed

+41
-65
lines changed

5 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
@@ -951,6 +951,13 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
951951
final def isInitialized: Boolean =
952952
validTo != NoPeriod
953953

954+
/** Some completers call sym.setInfo when still in-flight and then proceed with initialization (e.g. see LazyPackageType)
955+
* setInfo sets _validTo to current period, which means that after a call to setInfo isInitialized will start returning true.
956+
* Unfortunately, this doesn't mean that info becomes ready to be used, because subsequent initialization might change the info.
957+
* Therefore we need this method to distinguish between initialized and really initialized symbol states.
958+
*/
959+
final def isFullyInitialized: Boolean = _validTo != NoPeriod && (flags & LOCKED) == 0
960+
954961
/** Can this symbol be loaded by a reflective mirror?
955962
*
956963
* Scalac relies on `ScalaSignature' annotation to retain symbols across compilation runs.
@@ -3129,8 +3136,8 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
31293136
override def thisType: Type = {
31303137
val period = thisTypePeriod
31313138
if (period != currentPeriod) {
3132-
thisTypePeriod = currentPeriod
31333139
if (!isValid(period)) thisTypeCache = ThisType(this)
3140+
thisTypePeriod = currentPeriod
31343141
}
31353142
thisTypeCache
31363143
}
@@ -3218,9 +3225,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
32183225
override def typeOfThis = {
32193226
val period = typeOfThisPeriod
32203227
if (period != currentPeriod) {
3221-
typeOfThisPeriod = currentPeriod
32223228
if (!isValid(period))
32233229
typeOfThisCache = singleType(owner.thisType, sourceModule)
3230+
typeOfThisPeriod = currentPeriod
32243231
}
32253232
typeOfThisCache
32263233
}
@@ -3231,9 +3238,9 @@ trait Symbols extends api.Symbols { self: SymbolTable =>
32313238
// Skip a package object class, because the members are also in
32323239
// the package and we wish to avoid spurious ambiguities as in pos/t3999.
32333240
if (!isPackageObjectClass) {
3241+
implicitMembersCacheValue = tp.implicitMembers
32343242
implicitMembersCacheKey1 = tp
32353243
implicitMembersCacheKey2 = tp.decls.elems
3236-
implicitMembersCacheValue = tp.implicitMembers
32373244
}
32383245
}
32393246
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
@@ -44,7 +44,7 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable
4444
// we can keep this lock fine-grained, because methods of Scope don't do anything extraordinary, which makes deadlocks impossible
4545
// fancy subclasses of internal.Scopes#Scope should do synchronization themselves (e.g. see PackageScope for an example)
4646
private lazy val syncLock = new Object
47-
def syncLockSynchronized[T](body: => T): T = syncLock.synchronized { body }
47+
def syncLockSynchronized[T](body: => T): T = if (isCompilerUniverse) body else syncLock.synchronized { body }
4848
override def isEmpty: Boolean = syncLockSynchronized { super.isEmpty }
4949
override def size: Int = syncLockSynchronized { super.size }
5050
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

test/files/run/t6240-universe-code-gen.check

Whitespace-only changes.

0 commit comments

Comments
 (0)