diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2013-08-22 12:57:27 +0200 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2013-08-22 12:57:27 +0200 |
commit | d3c8a0bc81ea45177cee9a487c2c1739dbbdafff (patch) | |
tree | 8b9c997c764fcd38363ab9ad7936c49c9eec8f1d /src/reflect | |
parent | 3f72bed01d5054f77a27f69dff786463995e5450 (diff) | |
download | scala-d3c8a0bc81ea45177cee9a487c2c1739dbbdafff.tar.gz scala-d3c8a0bc81ea45177cee9a487c2c1739dbbdafff.tar.bz2 scala-d3c8a0bc81ea45177cee9a487c2c1739dbbdafff.zip |
SI-6240 Synchronizes Names
Previously we didn't have all possible name creation facilities covered
with locks, so some of them silently misbehaved and caused much grief:
http://groups.google.com/group/scala-internals/browse_thread/thread/ec1d3e2c4bcb000a.
This patch gets all the name factories under control.
Rather than relying on subclasses to override mutating methods
and add synchronization, they should instead override `synchronizeNames`
to return true and leave the placement of the locks up to
`reflect.internal.Names`.
These locks are placed around sections that mutate `typeHashtable`
and `termHashtable`.
This is done in the reflection universe, and in the interactive
compiler. The latter change will obviate an incomplete attempt
do to the same in `ScalaPresentationCompiler` in the scala-ide
project.
Diffstat (limited to 'src/reflect')
-rw-r--r-- | src/reflect/scala/reflect/internal/Names.scala | 85 | ||||
-rw-r--r-- | src/reflect/scala/reflect/runtime/SynchronizedOps.scala | 5 |
2 files changed, 57 insertions, 33 deletions
diff --git a/src/reflect/scala/reflect/internal/Names.scala b/src/reflect/scala/reflect/internal/Names.scala index 0d78e24548..b5116f8b67 100644 --- a/src/reflect/scala/reflect/internal/Names.scala +++ b/src/reflect/scala/reflect/internal/Names.scala @@ -18,6 +18,18 @@ trait Names extends api.Names { final val nameDebug = false + // Ideally we would just synchronize unconditionally and let HotSpot's Biased Locking + // kick in in the compiler universe, where access to the lock is single threaded. But, + // objects created in the first 4seconds of the JVM startup aren't eligible for biased + // locking. + // + // We might also be able to accept the performance hit, but we don't have tools to + // detect performance regressions. + // + // Discussion: https://groups.google.com/forum/#!search/biased$20scala-internals/scala-internals/0cYB7SkJ-nM/47MLhsgw8jwJ + protected def synchronizeNames: Boolean = false + private val nameLock: Object = new {} + /** Memory to store all names sequentially. */ var chrs: Array[Char] = new Array[Char](NAME_SIZE) private var nc = 0 @@ -76,22 +88,26 @@ trait Names extends api.Names { * string is not at the very end.) */ protected def newTermName(cs: Array[Char], offset: Int, len: Int, cachedString: String): TermName = { - val h = hashValue(cs, offset, len) & HASH_MASK - var n = termHashtable(h) - while ((n ne null) && (n.length != len || !equals(n.start, cs, offset, len))) - n = n.next - - if (n ne null) n - else { - // The logic order here is future-proofing against the possibility - // that name.toString will become an eager val, in which case the call - // to enterChars cannot follow the construction of the TermName. - val ncStart = nc - enterChars(cs, offset, len) - if (cachedString ne null) new TermName_S(ncStart, len, h, cachedString) - else new TermName_R(ncStart, len, h) + def body = { + val h = hashValue(cs, offset, len) & HASH_MASK + var n = termHashtable(h) + while ((n ne null) && (n.length != len || !equals(n.start, cs, offset, len))) + n = n.next + + if (n ne null) n + else { + // The logic order here is future-proofing against the possibility + // that name.toString will become an eager val, in which case the call + // to enterChars cannot follow the construction of the TermName. + val ncStart = nc + enterChars(cs, offset, len) + if (cachedString ne null) new TermName_S(ncStart, len, h, cachedString) + else new TermName_R(ncStart, len, h) + } } + if (synchronizeNames) nameLock.synchronized(body) else body } + protected def newTypeName(cs: Array[Char], offset: Int, len: Int, cachedString: String): TypeName = newTermName(cs, offset, len, cachedString).toTypeName @@ -128,6 +144,8 @@ trait Names extends api.Names { * For those of METHOD sort, its descriptor is stored ie has a leading '(' * * can-multi-thread + * TODO SI-6240 !!! JZ Really? the constructors TermName and TypeName publish unconstructed `this` references + * into the hash tables; we could observe them here before the subclass constructor completes. */ final def lookupTypeName(cs: Array[Char]): TypeName = { lookupTypeNameIfExisting(cs, true) } @@ -494,23 +512,26 @@ trait Names extends api.Names { override def toString = new String(chrs, index, len) } + // SYNCNOTE: caller to constructor must synchronize if `synchronizeNames` is enabled sealed abstract class TermName(index0: Int, len0: Int, hash: Int) extends Name(index0, len0) { type ThisNameType = TermName protected[this] def thisName: TermName = this - val next: TermName = termHashtable(hash) termHashtable(hash) = this def isTermName: Boolean = true def isTypeName: Boolean = false def toTermName: TermName = this def toTypeName: TypeName = { - val h = hashValue(chrs, index, len) & HASH_MASK - var n = typeHashtable(h) - while ((n ne null) && n.start != index) - n = n.next - - if (n ne null) n - else createCompanionName(h) + def body = { + val h = hashValue(chrs, index, len) & HASH_MASK + var n = typeHashtable(h) + while ((n ne null) && n.start != index) + n = n.next + + if (n ne null) n + else createCompanionName(h) + } + if (synchronizeNames) nameLock.synchronized(body) else body } def newName(str: String): TermName = newTermName(str) def companionName: TypeName = toTypeName @@ -518,6 +539,7 @@ trait Names extends api.Names { newTermName(chrs, start + from, to - from) def nameKind = "term" + /** SYNCNOTE: caller must synchronize if `synchronizeNames` is enabled */ protected def createCompanionName(h: Int): TypeName } @@ -534,16 +556,20 @@ trait Names extends api.Names { val next: TypeName = typeHashtable(hash) typeHashtable(hash) = this + def isTermName: Boolean = false def isTypeName: Boolean = true def toTermName: TermName = { - val h = hashValue(chrs, index, len) & HASH_MASK - var n = termHashtable(h) - while ((n ne null) && n.start != index) - n = n.next - - if (n ne null) n - else createCompanionName(h) + def body = { + val h = hashValue(chrs, index, len) & HASH_MASK + var n = termHashtable(h) + while ((n ne null) && n.start != index) + n = n.next + + if (n ne null) n + else createCompanionName(h) + } + if (synchronizeNames) nameLock.synchronized(body) else body } def toTypeName: TypeName = this def newName(str: String): TypeName = newTypeName(str) @@ -553,6 +579,7 @@ trait Names extends api.Names { def nameKind = "type" override def decode = if (nameDebug) super.decode + "!" else super.decode + /** SYNCNOTE: caller must synchronize if `synchronizeNames` is enabled */ protected def createCompanionName(h: Int): TermName } diff --git a/src/reflect/scala/reflect/runtime/SynchronizedOps.scala b/src/reflect/scala/reflect/runtime/SynchronizedOps.scala index 132470b2e7..6aa47a0405 100644 --- a/src/reflect/scala/reflect/runtime/SynchronizedOps.scala +++ b/src/reflect/scala/reflect/runtime/SynchronizedOps.scala @@ -9,10 +9,7 @@ private[reflect] trait SynchronizedOps extends internal.SymbolTable // Names - private lazy val nameLock = new Object - - override def newTermName(s: String): TermName = nameLock.synchronized { super.newTermName(s) } - override def newTypeName(s: String): TypeName = nameLock.synchronized { super.newTypeName(s) } + override protected def synchronizeNames = true // BaseTypeSeqs |