runtime: refactor insertion slot tracking for fast hashmap functions

* Avoid calculating insertk until needed.
* Avoid a pointer into b.tophash and just track the insertion index.
  This avoids b.tophash being marked as escaping to heap.
* Calculate val only once at the end of the mapassign functions.

Function sizes decrease slightly, e.g. for mapassign_faststr:
before "".mapassign_faststr STEXT size=1166 args=0x28 locals=0x78
after  "".mapassign_faststr STEXT size=1080 args=0x28 locals=0x68

name                     old time/op  new time/op  delta
MapAssign/Int32/256-4    19.4ns ± 4%  19.5ns ±11%     ~     (p=0.973 n=20+20)
MapAssign/Int32/65536-4  32.5ns ± 2%  32.4ns ± 3%     ~     (p=0.078 n=20+19)
MapAssign/Int64/256-4    20.3ns ± 6%  17.6ns ± 5%  -13.01%  (p=0.000 n=20+20)
MapAssign/Int64/65536-4  33.3ns ± 2%  33.3ns ± 1%     ~     (p=0.444 n=20+20)
MapAssign/Str/256-4      22.3ns ± 3%  22.4ns ± 3%     ~     (p=0.343 n=20+20)
MapAssign/Str/65536-4    44.9ns ± 1%  43.9ns ± 1%   -2.39%  (p=0.000 n=20+19)

Change-Id: I2627bb8a961d366d9473b5922fa129176319eb22
Reviewed-on: https://go-review.googlesource.com/74870
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/runtime/hashmap_fast.go b/src/runtime/hashmap_fast.go
index 2f582ee9b5dd49caf07f4889605ea45b2a3c4a41..4dc876fb1d7681600c0033936d157321ee108523 100644 (file)
--- a/src/runtime/hashmap_fast.go
+++ b/src/runtime/hashmap_fast.go
@@ -374,16 +374,16 @@ again:
        }
        b := (*bmap)(unsafe.Pointer(uintptr(h.buckets) + bucket*uintptr(t.bucketsize)))
 
-       var inserti *uint8
+       var insertb *bmap
+       var inserti uintptr
        var insertk unsafe.Pointer
-       var val unsafe.Pointer
+
        for {
                for i := uintptr(0); i < bucketCnt; i++ {
                        if b.tophash[i] == empty {
-                               if inserti == nil {
-                                       inserti = &b.tophash[i]
-                                       insertk = add(unsafe.Pointer(b), dataOffset+i*4)
-                                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.valuesize))
+                               if insertb == nil {
+                                       inserti = i
+                                       insertb = b
                                }
                                continue
                        }
@@ -391,7 +391,8 @@ again:
                        if k != key {
                                continue
                        }
-                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.valuesize))
+                       inserti = i
+                       insertb = b
                        goto done
                }
                ovf := b.overflow(t)
@@ -410,14 +411,14 @@ again:
                goto again // Growing the table invalidates everything, so try again
        }
 
-       if inserti == nil {
+       if insertb == nil {
                // all current buckets are full, allocate a new one.
-               newb := h.newoverflow(t, b)
-               inserti = &newb.tophash[0]
-               insertk = add(unsafe.Pointer(newb), dataOffset)
-               val = add(insertk, bucketCnt*4)
+               insertb = h.newoverflow(t, b)
+               inserti = 0 // not necessary, but avoids needlessly spilling inserti
        }
+       insertb.tophash[inserti&(bucketCnt-1)] = tophash(hash) // mask inserti to avoid bounds checks
 
+       insertk = add(unsafe.Pointer(insertb), dataOffset+inserti*4)
        // store new key at insert position
        if sys.PtrSize == 4 && t.key.kind&kindNoPointers == 0 && writeBarrier.enabled {
                writebarrierptr((*uintptr)(insertk), uintptr(key))
@@ -425,10 +426,10 @@ again:
                *(*uint32)(insertk) = key
        }
 
-       *inserti = tophash(hash)
        h.count++
 
 done:
+       val := add(unsafe.Pointer(insertb), dataOffset+bucketCnt*4+inserti*uintptr(t.valuesize))
        if h.flags&hashWriting == 0 {
                throw("concurrent map writes")
        }
@@ -463,16 +464,16 @@ again:
        }
        b := (*bmap)(unsafe.Pointer(uintptr(h.buckets) + bucket*uintptr(t.bucketsize)))
 
-       var inserti *uint8
+       var insertb *bmap
+       var inserti uintptr
        var insertk unsafe.Pointer
-       var val unsafe.Pointer
+
        for {
                for i := uintptr(0); i < bucketCnt; i++ {
                        if b.tophash[i] == empty {
-                               if inserti == nil {
-                                       inserti = &b.tophash[i]
-                                       insertk = add(unsafe.Pointer(b), dataOffset+i*8)
-                                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.valuesize))
+                               if insertb == nil {
+                                       insertb = b
+                                       inserti = i
                                }
                                continue
                        }
@@ -480,7 +481,8 @@ again:
                        if k != key {
                                continue
                        }
-                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.valuesize))
+                       insertb = b
+                       inserti = i
                        goto done
                }
                ovf := b.overflow(t)
@@ -499,14 +501,14 @@ again:
                goto again // Growing the table invalidates everything, so try again
        }
 
-       if inserti == nil {
+       if insertb == nil {
                // all current buckets are full, allocate a new one.
-               newb := h.newoverflow(t, b)
-               inserti = &newb.tophash[0]
-               insertk = add(unsafe.Pointer(newb), dataOffset)
-               val = add(insertk, bucketCnt*8)
+               insertb = h.newoverflow(t, b)
+               inserti = 0 // not necessary, but avoids needlessly spilling inserti
        }
+       insertb.tophash[inserti&(bucketCnt-1)] = tophash(hash) // mask inserti to avoid bounds checks
 
+       insertk = add(unsafe.Pointer(insertb), dataOffset+inserti*8)
        // store new key at insert position
        if t.key.kind&kindNoPointers == 0 && writeBarrier.enabled {
                if sys.PtrSize == 8 {
@@ -520,10 +522,10 @@ again:
                *(*uint64)(insertk) = key
        }
 
-       *inserti = tophash(hash)
        h.count++
 
 done:
+       val := add(unsafe.Pointer(insertb), dataOffset+bucketCnt*8+inserti*uintptr(t.valuesize))
        if h.flags&hashWriting == 0 {
                throw("concurrent map writes")
        }
@@ -531,7 +533,7 @@ done:
        return val
 }
 
-func mapassign_faststr(t *maptype, h *hmap, ky string) unsafe.Pointer {
+func mapassign_faststr(t *maptype, h *hmap, s string) unsafe.Pointer {
        if h == nil {
                panic(plainError("assignment to entry in nil map"))
        }
@@ -542,8 +544,8 @@ func mapassign_faststr(t *maptype, h *hmap, ky string) unsafe.Pointer {
        if h.flags&hashWriting != 0 {
                throw("concurrent map writes")
        }
-       key := stringStructOf(&ky)
-       hash := t.key.alg.hash(noescape(unsafe.Pointer(&ky)), uintptr(h.hash0))
+       key := stringStructOf(&s)
+       hash := t.key.alg.hash(noescape(unsafe.Pointer(&s)), uintptr(h.hash0))
 
        // Set hashWriting after calling alg.hash for consistency with mapassign.
        h.flags |= hashWriting
@@ -560,16 +562,16 @@ again:
        b := (*bmap)(unsafe.Pointer(uintptr(h.buckets) + bucket*uintptr(t.bucketsize)))
        top := tophash(hash)
 
-       var inserti *uint8
+       var insertb *bmap
+       var inserti uintptr
        var insertk unsafe.Pointer
-       var val unsafe.Pointer
+
        for {
                for i := uintptr(0); i < bucketCnt; i++ {
                        if b.tophash[i] != top {
-                               if b.tophash[i] == empty && inserti == nil {
-                                       inserti = &b.tophash[i]
-                                       insertk = add(unsafe.Pointer(b), dataOffset+i*2*sys.PtrSize)
-                                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*2*sys.PtrSize+i*uintptr(t.valuesize))
+                               if b.tophash[i] == empty && insertb == nil {
+                                       insertb = b
+                                       inserti = i
                                }
                                continue
                        }
@@ -581,7 +583,8 @@ again:
                                continue
                        }
                        // already have a mapping for key. Update it.
-                       val = add(unsafe.Pointer(b), dataOffset+bucketCnt*2*sys.PtrSize+i*uintptr(t.valuesize))
+                       inserti = i
+                       insertb = b
                        goto done
                }
                ovf := b.overflow(t)
@@ -600,20 +603,20 @@ again:
                goto again // Growing the table invalidates everything, so try again
        }
 
-       if inserti == nil {
+       if insertb == nil {
                // all current buckets are full, allocate a new one.
-               newb := h.newoverflow(t, b)
-               inserti = &newb.tophash[0]
-               insertk = add(unsafe.Pointer(newb), dataOffset)
-               val = add(insertk, bucketCnt*2*sys.PtrSize)
+               insertb = h.newoverflow(t, b)
+               inserti = 0 // not necessary, but avoids needlessly spilling inserti
        }
+       insertb.tophash[inserti&(bucketCnt-1)] = top // mask inserti to avoid bounds checks
 
+       insertk = add(unsafe.Pointer(insertb), dataOffset+inserti*2*sys.PtrSize)
        // store new key at insert position
        *((*stringStruct)(insertk)) = *key
-       *inserti = top
        h.count++
 
 done:
+       val := add(unsafe.Pointer(insertb), dataOffset+bucketCnt*2*sys.PtrSize+inserti*uintptr(t.valuesize))
        if h.flags&hashWriting == 0 {
                throw("concurrent map writes")
        }