From f979d072d339a24e4938d46588c153587d61af19 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Martin=20M=C3=B6hrmann?= <moehrmann@google.com>
Date: Sun, 3 May 2020 16:26:05 +0200
Subject: [PATCH] runtime: avoid memclr call for keys in mapdelete_fast
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Replace memclrHasPointers calls for keys in mapdelete_fast*
functions with direct writes since the key sizes are known
at compile time.

name                     old time/op  new time/op  delta
MapDelete/Pointer/100    33.7ns ± 1%  23.7ns ± 2%  -29.68%  (p=0.000 n=7+9)
MapDelete/Pointer/1000   41.6ns ± 5%  34.9ns ± 4%  -16.01%  (p=0.000 n=9+10)
MapDelete/Pointer/10000  45.6ns ± 1%  38.2ns ± 2%  -16.34%  (p=0.000 n=8+10)

Change-Id: Icaac43b520b93c2cf9fd192b822fae7203a7bbf7
Reviewed-on: https://go-review.googlesource.com/c/go/+/231737
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
---
 src/runtime/map_fast32.go |  8 ++++++--
 src/runtime/map_fast64.go |  8 +++++++-
 src/runtime/map_test.go   | 22 ++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/runtime/map_fast32.go b/src/runtime/map_fast32.go
index 534454f3ad..d035ed0386 100644
--- a/src/runtime/map_fast32.go
+++ b/src/runtime/map_fast32.go
@@ -299,8 +299,12 @@ search:
 				continue
 			}
 			// Only clear key if there are pointers in it.
-			if t.key.ptrdata != 0 {
-				memclrHasPointers(k, t.key.size)
+			// This can only happen if pointers are 32 bit
+			// wide as 64 bit pointers do not fit into a 32 bit key.
+			if sys.PtrSize == 4 && t.key.ptrdata != 0 {
+				// The key must be a pointer as we checked pointers are
+				// 32 bits wide and the key is 32 bits wide also.
+				*(*unsafe.Pointer)(k) = nil
 			}
 			e := add(unsafe.Pointer(b), dataOffset+bucketCnt*4+i*uintptr(t.elemsize))
 			if t.elem.ptrdata != 0 {
diff --git a/src/runtime/map_fast64.go b/src/runtime/map_fast64.go
index 1669c7cfe9..f1f3927598 100644
--- a/src/runtime/map_fast64.go
+++ b/src/runtime/map_fast64.go
@@ -300,7 +300,13 @@ search:
 			}
 			// Only clear key if there are pointers in it.
 			if t.key.ptrdata != 0 {
-				memclrHasPointers(k, t.key.size)
+				if sys.PtrSize == 8 {
+					*(*unsafe.Pointer)(k) = nil
+				} else {
+					// There are three ways to squeeze at one ore more 32 bit pointers into 64 bits.
+					// Just call memclrHasPointers instead of trying to handle all cases here.
+					memclrHasPointers(k, 8)
+				}
 			}
 			e := add(unsafe.Pointer(b), dataOffset+bucketCnt*8+i*uintptr(t.elemsize))
 			if t.elem.ptrdata != 0 {
diff --git a/src/runtime/map_test.go b/src/runtime/map_test.go
index 1b7ccad6ed..302b3c23c1 100644
--- a/src/runtime/map_test.go
+++ b/src/runtime/map_test.go
@@ -993,6 +993,27 @@ func benchmarkMapDeleteStr(b *testing.B, n int) {
 	}
 }
 
+func benchmarkMapDeletePointer(b *testing.B, n int) {
+	i2p := make([]*int, n)
+	for i := 0; i < n; i++ {
+		i2p[i] = new(int)
+	}
+	a := make(map[*int]int, n)
+	b.ResetTimer()
+	k := 0
+	for i := 0; i < b.N; i++ {
+		if len(a) == 0 {
+			b.StopTimer()
+			for j := 0; j < n; j++ {
+				a[i2p[j]] = j
+			}
+			k = i
+			b.StartTimer()
+		}
+		delete(a, i2p[i-k])
+	}
+}
+
 func runWith(f func(*testing.B, int), v ...int) func(*testing.B) {
 	return func(b *testing.B) {
 		for _, n := range v {
@@ -1023,6 +1044,7 @@ func BenchmarkMapDelete(b *testing.B) {
 	b.Run("Int32", runWith(benchmarkMapDeleteInt32, 100, 1000, 10000))
 	b.Run("Int64", runWith(benchmarkMapDeleteInt64, 100, 1000, 10000))
 	b.Run("Str", runWith(benchmarkMapDeleteStr, 100, 1000, 10000))
+	b.Run("Pointer", runWith(benchmarkMapDeletePointer, 100, 1000, 10000))
 }
 
 func TestDeferDeleteSlow(t *testing.T) {
-- 
2.50.0