From: Keith Randall <khr@golang.org>
Date: Tue, 9 Sep 2014 00:42:21 +0000 (-0700)
Subject: runtime: on bigger maps, start iterator at a random bucket.
X-Git-Tag: go1.4beta1~465
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=55c458e05f35d0d5d539107da07b744ad96f268e;p=gostls13.git

runtime: on bigger maps, start iterator at a random bucket.

This change brings the iter/delete pattern down to O(n lgn) from O(n^2).

Fixes #8412.

before:
BenchmarkMapPop100	   50000	     32498 ns/op
BenchmarkMapPop1000	     500	   3244851 ns/op
BenchmarkMapPop10000	       5	 270276855 ns/op

after:
BenchmarkMapPop100	  100000	     16169 ns/op
BenchmarkMapPop1000	    5000	    300416 ns/op
BenchmarkMapPop10000	     300	   5990814 ns/op

LGTM=iant
R=golang-codereviews, iant, khr
CC=golang-codereviews
https://golang.org/cl/141270043
---

diff --git a/src/runtime/hashmap.go b/src/runtime/hashmap.go
index 55287f6ff9..cbcc6c4041 100644
--- a/src/runtime/hashmap.go
+++ b/src/runtime/hashmap.go
@@ -134,11 +134,12 @@ type hiter struct {
 	h           *hmap
 	buckets     unsafe.Pointer // bucket ptr at hash_iter initialization time
 	bptr        *bmap          // current bucket
+	startBucket uintptr        // bucket iteration started at
 	offset      uint8          // intra-bucket offset to start from during iteration (should be big enough to hold bucketCnt-1)
-	done        bool
+	wrapped     bool           // already wrapped around from end of bucket array to beginning
 	B           uint8
+	i	    uint8
 	bucket      uintptr
-	i           uintptr
 	checkBucket uintptr
 }
 
@@ -560,10 +561,22 @@ func mapiterinit(t *maptype, h *hmap, it *hiter) {
 	it.B = h.B
 	it.buckets = h.buckets
 
+	// decide where to start
+	switch {
+	case h.B == 0:
+		it.startBucket = 0
+		it.offset = uint8(fastrand1()) & (bucketCnt - 1)
+	case h.B <= 31:
+		it.startBucket = uintptr(fastrand1()) & (uintptr(1)<<h.B-1)
+		it.offset = 0
+	default:
+		it.startBucket = (uintptr(fastrand1()) + uintptr(fastrand1())<<31) & (uintptr(1)<<h.B-1)
+		it.offset = 0
+	}
+
 	// iterator state
-	it.bucket = 0
-	it.offset = uint8(fastrand1() & (bucketCnt - 1))
-	it.done = false
+	it.bucket = it.startBucket
+	it.wrapped = false
 	it.bptr = nil
 
 	// Remember we have an iterator.
@@ -596,7 +609,7 @@ func mapiternext(it *hiter) {
 
 next:
 	if b == nil {
-		if it.done {
+		if bucket == it.startBucket && it.wrapped {
 			// end of iteration
 			it.key = nil
 			it.value = nil
@@ -622,14 +635,14 @@ next:
 		bucket++
 		if bucket == uintptr(1)<<it.B {
 			bucket = 0
-			it.done = true
+			it.wrapped = true
 		}
 		i = 0
 	}
 	for ; i < bucketCnt; i++ {
-		offi := (i + uintptr(it.offset)) & (bucketCnt - 1)
-		k := add(unsafe.Pointer(b), dataOffset+offi*uintptr(t.keysize))
-		v := add(unsafe.Pointer(b), dataOffset+bucketCnt*uintptr(t.keysize)+offi*uintptr(t.valuesize))
+		offi := (i + it.offset) & (bucketCnt - 1)
+		k := add(unsafe.Pointer(b), dataOffset+uintptr(offi)*uintptr(t.keysize))
+		v := add(unsafe.Pointer(b), dataOffset+bucketCnt*uintptr(t.keysize)+uintptr(offi)*uintptr(t.valuesize))
 		if b.tophash[offi] != empty && b.tophash[offi] != evacuatedEmpty {
 			if checkBucket != noCheck {
 				// Special case: iterator was started during a grow and the
diff --git a/src/runtime/map_test.go b/src/runtime/map_test.go
index 8bedc05689..2e87a94a03 100644
--- a/src/runtime/map_test.go
+++ b/src/runtime/map_test.go
@@ -475,3 +475,24 @@ func TestMapStringBytesLookup(t *testing.T) {
 		t.Errorf("AllocsPerRun for x,ok = m[string(buf)] = %v, want 0", n)
 	}
 }
+
+func benchmarkMapPop(b *testing.B, n int) {
+	m := map[int]int{}
+	for i := 0; i < b.N; i++ {
+		for j := 0; j < n; j++ {
+			m[j] = j
+		}
+		for j := 0; j < n; j++ {
+			// Use iterator to pop an element.
+			// We want this to be fast, see issue 8412.
+			for k := range m {
+				delete(m, k)
+				break
+			}
+		}
+	}
+}
+
+func BenchmarkMapPop100(b *testing.B)   { benchmarkMapPop(b, 100) }
+func BenchmarkMapPop1000(b *testing.B)  { benchmarkMapPop(b, 1000) }
+func BenchmarkMapPop10000(b *testing.B) { benchmarkMapPop(b, 10000) }