Use unorderedHash, LinkedHashMap/Set are equals to other Maps

lrytz · lrytz · commit 2b2806add6cb · 2022-12-22T13:41:27.000+01:00
Plus review cleanups. Reuse a single iterator implementation.
diff --git a/library/src/scala/collection/mutable/HashMap.scala b/library/src/scala/collection/mutable/HashMap.scala
@@ -109,12 +109,12 @@ class HashMap[K, V](initialCapacity: Int, loadFactor: Double)
         }
         this
       case lhm: mutable.LinkedHashMap[K, V] =>
-      val iter = lhm.entryIterator
-      while (iter.hasNext) {
-        val entry = iter.next()
-        put0(entry.key, entry.value,entry.hash,getOld = false)
-      }
-      this
+        val iter = lhm.entryIterator
+        while (iter.hasNext) {
+          val entry = iter.next()
+          put0(entry.key, entry.value, entry.hash, getOld = false)
+        }
+        this
       case thatMap: Map[K, V] =>
         thatMap.foreachEntry { (key: K, value: V) =>
           put0(key, value, improveHash(key.##), getOld = false)
diff --git a/library/src/scala/collection/mutable/LinkedHashMap.scala b/library/src/scala/collection/mutable/LinkedHashMap.scala
@@ -93,7 +93,7 @@ class LinkedHashMap[K, V]
   }
   override def sizeHint(size: Int): Unit = {
     val target = tableSizeFor(((size + 1).toDouble / LinkedHashMap.defaultLoadFactor).toInt)
-    if(target > table.length) growTable(target)
+    if (target > table.length) growTable(target)
   }
 
   override def contains(key: K): Boolean = {
@@ -131,23 +131,23 @@ class LinkedHashMap[K, V]
       // subclasses of LinkedHashMap might customise `get` ...
       super.getOrElseUpdate(key, defaultValue)
     } else {
-          val hash = computeHash(key)
-          val idx = index(hash)
-          val nd = table(idx) match {
-            case null => null
-            case nd => nd.findEntry(key, hash)
-          }
-          if (nd != null) nd.value
-          else {
-            val table0 = table
-            val default = defaultValue
-            if (contentSize + 1 >= threshold) growTable(table.length * 2)
-            // Avoid recomputing index if the `defaultValue()` or new element hasn't triggered a table resize.
-            val newIdx = if (table0 eq table) idx else index(hash)
-            put0(key, default, false, hash, newIdx)
-            default
-          }
-        }
+      val hash = computeHash(key)
+      val idx = index(hash)
+      val nd = table(idx) match {
+        case null => null
+        case nd => nd.findEntry(key, hash)
+      }
+      if (nd != null) nd.value
+      else {
+        val table0 = table
+        val default = defaultValue
+        if (contentSize + 1 >= threshold) growTable(table.length * 2)
+        // Avoid recomputing index if the `defaultValue()` or new element hasn't triggered a table resize.
+        val newIdx = if (table0 eq table) idx else index(hash)
+        put0(key, default, false, hash, newIdx)
+        default
+      }
+    }
   }
 
   private[this] def removeEntry0(elem: K): Entry = removeEntry0(elem, computeHash(elem))
@@ -215,104 +215,103 @@ class LinkedHashMap[K, V]
     this
   }
 
-  def iterator: Iterator[(K, V)] = new AbstractIterator[(K, V)] {
+  private[this] abstract class LinkedHashMapIterator[T] extends AbstractIterator[T] {
     private[this] var cur = firstEntry
-    def hasNext = cur ne null
-    def next() =
-      if (hasNext) { val res = (cur.key, cur.value); cur = cur.later; res }
+    def extract(nd: Entry): T
+    def hasNext: Boolean = cur ne null
+    def next(): T =
+      if (hasNext) { val r = extract(cur); cur = cur.later; r }
       else Iterator.empty.next()
   }
 
+  def iterator: Iterator[(K, V)] =
+    if (size == 0) Iterator.empty
+    else new LinkedHashMapIterator[(K, V)] {
+      def extract(nd: Entry): (K, V) = (nd.key, nd.value)
+    }
+
   protected class LinkedKeySet extends KeySet {
     override def iterableFactory: IterableFactory[collection.Set] = LinkedHashSet
   }
 
   override def keySet: collection.Set[K] = new LinkedKeySet
 
-  override def keysIterator: Iterator[K] = new AbstractIterator[K] {
-    private[this] var cur = firstEntry
-    def hasNext = cur ne null
-    def next() =
-      if (hasNext) { val res = cur.key; cur = cur.later; res }
-      else Iterator.empty.next()
-  }
+  override def keysIterator: Iterator[K] =
+    if (size == 0) Iterator.empty
+    else new LinkedHashMapIterator[K] {
+      def extract(nd: Entry): K = nd.key
+    }
 
-  private[collection] def entryIterator: Iterator[Entry] = new AbstractIterator[Entry] {
-    private[this] var cur = firstEntry
+  private[collection] def entryIterator: Iterator[Entry] =
+    if (size == 0) Iterator.empty
+    else new LinkedHashMapIterator[Entry] {
+      def extract(nd: Entry): Entry = nd
+    }
 
-    def hasNext = cur ne null
 
-    def next() =
-      if (hasNext) {
-        val res = cur; cur = cur.later; res
-      }
-      else Iterator.empty.next()
-  }
   // Override updateWith for performance, so we can do the update while hashing
   // the input key only once and performing one lookup into the hash table
   override def updateWith(key: K)(remappingFunction: Option[V] => Option[V]): Option[V] = {
     if (getClass != classOf[LinkedHashMap[_, _]]) {
       // subclasses of LinkedHashMap might customise `get` ...
       super.updateWith(key)(remappingFunction)
     } else {
-          val hash = computeHash(key)
-          val indexedHash = index(hash)
-
-          var foundEntry: Entry = null
-          var previousEntry: Entry = null
-          table(indexedHash) match {
-            case null =>
-            case nd =>
-              @tailrec
-              def findEntry(prev: Entry, nd: Entry, k: K, h: Int): Unit = {
-                if (h == nd.hash && k == nd.key) {
-                  previousEntry = prev
-                  foundEntry = nd
-                }
-                else if ((nd.next eq null) || (nd.hash > h)) ()
-                else findEntry(nd, nd.next, k, h)
-              }
-
-              findEntry(null, nd, key, hash)
+      val hash = computeHash(key)
+      val indexedHash = index(hash)
+
+      var foundEntry: Entry = null
+      var previousEntry: Entry = null
+      table(indexedHash) match {
+        case null =>
+        case nd =>
+          @tailrec
+          def findEntry(prev: Entry, nd: Entry, k: K, h: Int): Unit = {
+            if (h == nd.hash && k == nd.key) {
+              previousEntry = prev
+              foundEntry = nd
+            }
+            else if ((nd.next eq null) || (nd.hash > h)) ()
+            else findEntry(nd, nd.next, k, h)
           }
 
-          val previousValue = foundEntry match {
-            case null => None
-            case nd => Some(nd.value)
-          }
+          findEntry(null, nd, key, hash)
+      }
+
+      val previousValue = foundEntry match {
+        case null => None
+        case nd => Some(nd.value)
+      }
 
-          val nextValue = remappingFunction(previousValue)
+      val nextValue = remappingFunction(previousValue)
 
-          (previousValue, nextValue) match {
-            case (None, None) => // do nothing
+      (previousValue, nextValue) match {
+        case (None, None) => // do nothing
 
-            case (Some(_), None) =>
-              if (previousEntry != null) previousEntry.next = foundEntry.next
-              else table(indexedHash) = foundEntry.next
-              deleteEntry(foundEntry)
-              contentSize -= 1
+        case (Some(_), None) =>
+          if (previousEntry != null) previousEntry.next = foundEntry.next
+          else table(indexedHash) = foundEntry.next
+          deleteEntry(foundEntry)
+          contentSize -= 1
 
-            case (None, Some(value)) =>
-              val newIndexedHash =
-                if (contentSize + 1 >= threshold) {
-                  growTable(table.length * 2)
-                  index(hash)
-                } else indexedHash
-              put0(key, value, false, hash, newIndexedHash)
+        case (None, Some(value)) =>
+          val newIndexedHash =
+            if (contentSize + 1 >= threshold) {
+              growTable(table.length * 2)
+              index(hash)
+            } else indexedHash
+          put0(key, value, false, hash, newIndexedHash)
 
-            case (Some(_), Some(newValue)) => foundEntry.value = newValue
-          }
-          nextValue
+        case (Some(_), Some(newValue)) => foundEntry.value = newValue
+      }
+      nextValue
     }
   }
 
-  override def valuesIterator: Iterator[V] = new AbstractIterator[V] {
-    private[this] var cur = firstEntry
-    def hasNext = cur ne null
-    def next() =
-      if (hasNext) { val res = cur.value; cur = cur.later; res }
-      else Iterator.empty.next()
-  }
+  override def valuesIterator: Iterator[V] =
+    if (size == 0) Iterator.empty
+    else new LinkedHashMapIterator[V] {
+      def extract(nd: Entry): V = nd.value
+    }
 
 
   override def foreach[U](f: ((K, V)) => U): Unit = {
@@ -452,31 +451,18 @@ class LinkedHashMap[K, V]
     }
   }
 
-  override def hashCode(): Int = {
-    abstract class LinkedHashMapIterator[A](val firstentry: Entry) extends AbstractIterator[A] {
-    var cur = firstentry
-    def extract(nd: Entry): A
-    def hasNext: Boolean =  cur ne null
-    def next(): A =
-      if(hasNext) {
-        val r = extract(cur)
-        cur = cur.later
-        r
-      } else Iterator.empty.next()
-    }
-
+  override def hashCode: Int = {
     if (isEmpty) MurmurHash3.emptyMapHash
     else {
-      val tupleHashIterator = new LinkedHashMapIterator[Any](firstEntry) {
+      val tupleHashIterator = new LinkedHashMapIterator[Any] {
         var hash: Int = 0
         override def hashCode: Int = hash
-        override  def extract(nd: Entry): Any = {
+        override def extract(nd: Entry): Any = {
           hash = MurmurHash3.tuple2Hash(unimproveHash(nd.hash), nd.value.##)
           this
         }
       }
-
-      MurmurHash3.orderedHash(tupleHashIterator, MurmurHash3.mapSeed)
+      MurmurHash3.unorderedHash(tupleHashIterator, MurmurHash3.mapSeed)
     }
   }
   @nowarn("""cat=deprecation&origin=scala\.collection\.Iterable\.stringPrefix""")
diff --git a/library/src/scala/collection/mutable/LinkedHashSet.scala b/library/src/scala/collection/mutable/LinkedHashSet.scala
@@ -43,7 +43,7 @@ class LinkedHashSet[A]
   // stepper is not overridden to use XTableStepper because that stepper would not return the
   // elements in insertion order
 
-  type Entry = LinkedHashSet.Entry[A]
+  /*private*/ type Entry = LinkedHashSet.Entry[A]
 
   protected var firstEntry: Entry = null
 
@@ -100,23 +100,21 @@ class LinkedHashSet[A]
 
   override def remove(elem: A): Boolean = remove0(elem, computeHash(elem))
 
-  def iterator: Iterator[A] = new AbstractIterator[A] {
+  private[this] abstract class LinkedHashSetIterator[T] extends AbstractIterator[T] {
     private[this] var cur = firstEntry
-    def hasNext = cur ne null
-    def next() =
-      if (hasNext) { val res = cur.key; cur = cur.later; res }
+    def extract(nd: Entry): T
+    def hasNext: Boolean = cur ne null
+    def next(): T =
+      if (hasNext) { val r = extract(cur); cur = cur.later; r }
       else Iterator.empty.next()
   }
-  private[collection] def entryIterator: Iterator[Entry] = new AbstractIterator[Entry] {
-    private[this] var cur = firstEntry
 
-    def hasNext = cur ne null
+  def iterator: Iterator[A] = new LinkedHashSetIterator[A] {
+    override def extract(nd: Entry): A = nd.key
+  }
 
-    def next() =
-      if (hasNext) {
-        val res = cur; cur = cur.later; res
-      }
-      else Iterator.empty.next()
+  private[collection] def entryIterator: Iterator[Entry] = new LinkedHashSetIterator[Entry] {
+    override def extract(nd: Entry): Entry = nd
   }
 
   override def foreach[U](f: A => U): Unit = {
@@ -284,22 +282,11 @@ class LinkedHashSet[A]
     }
   }
 
-  override def hashCode(): Int = {
-      abstract class LinkedHashSetIterator[B](val firstentry: Entry) extends AbstractIterator[B] {
-        var cur = firstentry
-        def extract(nd: Entry): B
-        def hasNext: Boolean = cur ne null
-        def next(): B =
-          if (hasNext) {
-            val r = extract(cur)
-            cur = cur.later
-            r
-          } else Iterator.empty.next()
-      }
-
-      val setHashIterator = if (isEmpty) this.iterator
+  override def hashCode: Int = {
+    val setHashIterator =
+      if (isEmpty) this.iterator
       else {
-        new LinkedHashSetIterator[Any](firstEntry) {
+        new LinkedHashSetIterator[Any] {
           var hash: Int = 0
           override def hashCode: Int = hash
           override def extract(nd: Entry): Any = {
@@ -308,7 +295,7 @@ class LinkedHashSet[A]
           }
         }
       }
-      MurmurHash3.orderedHash(setHashIterator, MurmurHash3.setSeed)
+    MurmurHash3.unorderedHash(setHashIterator, MurmurHash3.setSeed)
   }
 
   @nowarn("""cat=deprecation&origin=scala\.collection\.Iterable\.stringPrefix""")
