From 9d80fd0148ba7466bbb47e661aea33ee930a0d32 Mon Sep 17 00:00:00 2001 From: Lukas Rytz Date: Fri, 16 May 2014 16:29:25 +0200 Subject: Cleanups in Names.scala Enter only fully constructed Name objects into the hash table, this should make lookupTypeName thread-safe. Clean up lookupTypeName - the hash code for a type name is the same as for its correspondent term name. Going from a type name toTermName should never create a new TermName instance. Assert that. --- src/reflect/scala/reflect/internal/Names.scala | 91 +++++++++++--------------- 1 file changed, 38 insertions(+), 53 deletions(-) (limited to 'src/reflect') diff --git a/src/reflect/scala/reflect/internal/Names.scala b/src/reflect/scala/reflect/internal/Names.scala index fce1fc2a0c..05c060e81f 100644 --- a/src/reflect/scala/reflect/internal/Names.scala +++ b/src/reflect/scala/reflect/internal/Names.scala @@ -109,8 +109,13 @@ trait Names extends api.Names { // 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) + val next = termHashtable(h) + val termName = + if (cachedString ne null) new TermName_S(ncStart, len, next, cachedString) + else new TermName_R(ncStart, len, next) + // Add the new termName to the hashtable only after it's been fully constructed + termHashtable(h) = termName + termName } } if (synchronizeNames) nameLock.synchronized(body) else body @@ -148,40 +153,20 @@ trait Names extends api.Names { newTermName(bs, offset, len).toTypeName /** - * Used only by the GenBCode backend, to represent bytecode-level types in a way that makes equals() and hashCode() efficient. - * For bytecode-level types of OBJECT sort, its internal name (not its descriptor) is stored. - * For those of ARRAY sort, its descriptor is stored ie has a leading '[' - * For those of METHOD sort, its descriptor is stored ie has a leading '(' + * Used by the GenBCode backend to lookup type names that are known to already exist. This method + * might be invoked in a multi-threaded setting. Invoking newTypeName instead might be unsafe. * - * 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. + * can-multi-thread: names are added to the hash tables only after they are fully constructed. */ - final def lookupTypeName(cs: Array[Char]): TypeName = { lookupTypeNameIfExisting(cs, true) } + final def lookupTypeName(cs: Array[Char]): TypeName = { + val hash = hashValue(cs, 0, cs.length) & HASH_MASK + var typeName = typeHashtable(hash) - final def lookupTypeNameIfExisting(cs: Array[Char], failOnNotFound: Boolean): TypeName = { - - val hterm = hashValue(cs, 0, cs.length) & HASH_MASK - var nterm = termHashtable(hterm) - while ((nterm ne null) && (nterm.length != cs.length || !equals(nterm.start, cs, 0, cs.length))) { - nterm = nterm.next - } - if (nterm eq null) { - if (failOnNotFound) { assert(false, "TermName not yet created: " + new String(cs)) } - return null - } - - val htype = hashValue(chrs, nterm.start, nterm.length) & HASH_MASK - var ntype = typeHashtable(htype) - while ((ntype ne null) && ntype.start != nterm.start) { - ntype = ntype.next - } - if (ntype eq null) { - if (failOnNotFound) { assert(false, "TypeName not yet created: " + new String(cs)) } - return null + while ((typeName ne null) && (typeName.length != cs.length || !equals(typeName.start, cs, 0, cs.length))) { + typeName = typeName.next } - - ntype + assert(typeName != null, s"TypeName ${new String(cs)} not yet created.") + typeName } // Classes ---------------------------------------------------------------------- @@ -518,43 +503,47 @@ trait Names extends api.Names { /** TermName_S and TypeName_S have fields containing the string version of the name. * TermName_R and TypeName_R recreate it each time toString is called. */ - private final class TermName_S(index0: Int, len0: Int, hash: Int, override val toString: String) extends TermName(index0, len0, hash) { - protected def createCompanionName(h: Int): TypeName = new TypeName_S(index, len, h, toString) + private final class TermName_S(index0: Int, len0: Int, next0: TermName, override val toString: String) extends TermName(index0, len0, next0) { + protected def createCompanionName(next: TypeName): TypeName = new TypeName_S(index, len, next, toString) override def newName(str: String): TermName = newTermNameCached(str) } - private final class TypeName_S(index0: Int, len0: Int, hash: Int, override val toString: String) extends TypeName(index0, len0, hash) { - protected def createCompanionName(h: Int): TermName = new TermName_S(index, len, h, toString) + private final class TypeName_S(index0: Int, len0: Int, next0: TypeName, override val toString: String) extends TypeName(index0, len0, next0) { override def newName(str: String): TypeName = newTypeNameCached(str) } - private final class TermName_R(index0: Int, len0: Int, hash: Int) extends TermName(index0, len0, hash) { - protected def createCompanionName(h: Int): TypeName = new TypeName_R(index, len, h) + private final class TermName_R(index0: Int, len0: Int, next0: TermName) extends TermName(index0, len0, next0) { + protected def createCompanionName(next: TypeName): TypeName = new TypeName_R(index, len, next) override def toString = new String(chrs, index, len) } - private final class TypeName_R(index0: Int, len0: Int, hash: Int) extends TypeName(index0, len0, hash) { - protected def createCompanionName(h: Int): TermName = new TermName_R(index, len, h) + private final class TypeName_R(index0: Int, len0: Int, next0: TypeName) extends TypeName(index0, len0, next0) { 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) with TermNameApi { + sealed abstract class TermName(index0: Int, len0: Int, val next: TermName) extends Name(index0, len0) with TermNameApi { 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 = { def body = { + // Re-computing the hash saves a field for storing it in the TermName 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) + else { + val next = typeHashtable(h) + val typeName = createCompanionName(next) + // Add the new typeName to the hashtable only after it's been fully constructed + typeHashtable(h) = typeName + typeName + } } if (synchronizeNames) nameLock.synchronized(body) else body } @@ -565,7 +554,7 @@ trait Names extends api.Names { def nameKind = "term" /** SYNCNOTE: caller must synchronize if `synchronizeNames` is enabled */ - protected def createCompanionName(h: Int): TypeName + protected def createCompanionName(next: TypeName): TypeName } implicit val TermNameTag = ClassTag[TermName](classOf[TermName]) @@ -575,24 +564,22 @@ trait Names extends api.Names { def unapply(name: TermName): Option[String] = Some(name.toString) } - sealed abstract class TypeName(index0: Int, len0: Int, hash: Int) extends Name(index0, len0) with TypeNameApi { + sealed abstract class TypeName(index0: Int, len0: Int, val next: TypeName) extends Name(index0, len0) with TypeNameApi { type ThisNameType = TypeName protected[this] def thisName: TypeName = this - val next: TypeName = typeHashtable(hash) - typeHashtable(hash) = this - def isTermName: Boolean = false def isTypeName: Boolean = true def toTermName: TermName = { def body = { + // Re-computing the hash saves a field for storing it in the TypeName 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) + assert (n ne null, s"TypeName $this is missing its correspondent") + n } if (synchronizeNames) nameLock.synchronized(body) else body } @@ -604,8 +591,6 @@ 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 } implicit val TypeNameTag = ClassTag[TypeName](classOf[TypeName]) -- cgit v1.2.3