Skip to content

Commit 3ad578e

Browse files
committed
Fix HashMap and HashSet remove criterion
Make criterion when to fill a hole in a HashMap or HashSet remove more robust. The old criterion relied on fill factor always being less than 0.5. The new criterion works for arbitrary fill factors.
1 parent ef59339 commit 3ad578e

File tree

3 files changed

+79
-4
lines changed

3 files changed

+79
-4
lines changed

compiler/src/dotty/tools/dotc/util/GenericHashMap.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,11 @@ abstract class GenericHashMap[Key, Value]
111111
k = keyAt(idx)
112112
k != null
113113
do
114+
val eidx = index(hash(k))
114115
if isDense
115-
|| index(hole - index(hash(k))) < limit * 2
116-
// hash(k) is then logically at or before hole; can be moved forward to fill hole
116+
|| index(eidx - (hole + 2)) > index(idx - (hole + 2))
117+
// entry `e` at `idx` can move unless `index(hash(e))` is in
118+
// the (ring-)interval [hole + 2 .. idx]
117119
then
118120
setKey(hole, k)
119121
setValue(hole, valueAt(idx))

compiler/src/dotty/tools/dotc/util/HashSet.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,11 @@ class HashSet[T](initialCapacity: Int = 8, capacityMultiple: Int = 2) extends Mu
109109
e = entryAt(idx)
110110
e != null
111111
do
112+
val eidx = index(hash(e))
112113
if isDense
113-
|| index(hole - index(hash(e))) < limit
114-
// hash(k) is then logically at or before hole; can be moved forward to fill hole
114+
|| index(eidx - (hole + 1)) > index(idx - (hole + 1))
115+
// entry `e` at `idx` can move unless `index(hash(e))` is in
116+
// the (ring-)interval [hole + 1 .. idx]
115117
then
116118
setEntry(hole, e)
117119
hole = idx

tests/run-with-compiler/settest.scala

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
trait Generator[+T]:
2+
self =>
3+
def generate: T
4+
def map[S](f: T => S) = new Generator[S]:
5+
def generate: S = f(self.generate)
6+
def flatMap[S](f: T => Generator[S]) = new Generator[S]:
7+
def generate: S = f(self.generate).generate
8+
9+
object Generator:
10+
val NumLimit = 300
11+
val Iterations = 10000
12+
13+
given integers as Generator[Int]:
14+
val rand = new java.util.Random
15+
def generate = rand.nextInt()
16+
17+
given booleans as Generator[Boolean] =
18+
integers.map(x => x > 0)
19+
20+
def range(end: Int): Generator[Int] =
21+
integers.map(x => (x % end).abs)
22+
23+
enum Op:
24+
case Lookup, Update, Remove
25+
export Op._
26+
27+
given ops as Generator[Op] =
28+
range(10).map {
29+
case 0 | 1 | 2 | 3 => Lookup
30+
case 4 | 5 | 6 | 7 => Update
31+
case 8 | 9 => Remove
32+
}
33+
34+
val nums: Generator[Integer] = range(NumLimit).map(Integer(_))
35+
36+
@main def Test =
37+
import Generator._
38+
39+
val set1 = dotty.tools.dotc.util.HashSet[Int]()
40+
val set2 = scala.collection.mutable.HashSet[Int]()
41+
42+
def checkSame() =
43+
assert(set1.size == set2.size)
44+
for e <- set1.iterator do
45+
assert(set2.contains(e))
46+
for e <- set2.iterator do
47+
assert(set1.contains(e))
48+
49+
def lookupTest(num: Integer) =
50+
val res1 = set1.contains(num)
51+
val res2 = set2.contains(num)
52+
assert(res1 == res2)
53+
54+
def updateTest(num: Integer) =
55+
lookupTest(num)
56+
set1 += num
57+
set2 += num
58+
checkSame()
59+
60+
def removeTest(num: Integer) =
61+
//println(s"test remove $num")
62+
set1 -= num
63+
set2 -= num
64+
checkSame()
65+
66+
for i <- 0 until Iterations do
67+
val num = nums.generate
68+
Generator.ops.generate match
69+
case Lookup => lookupTest(num)
70+
case Update => updateTest(num)
71+
case Remove => removeTest(num)

0 commit comments

Comments
 (0)