runtime: remove mapaccess2 in favour of mapaccess (or in reverse)

[mateusz834]

Given that mapaccess returns the zeroval pointer, i don't see a reason to maintain a second mapaccess2 function that also return a boolean. We could instead at the caller site use mapaccess and compute the ok boolean based on ptr == zeroval. This way we could eliminate mapaccess2 (1xnormal and 3xfast ones) implementations from the runtime.

Given that registers die at every CALL this might be also done in reverse, so remove mapaccess in favour of mapaccess2 (the asm diff is going to show only a additional MOV 1 BX) difference, between mapaccess/mapaccess2).

Maybe i am missing some details though. Any thoughts?

CC @prattmic

[mateusz834]

(obviously keeping a mapaccess2 stub for our beloved linknamers).

[mateusz834]

Benchmarking this manually (without any compiler/runtime change):

func makeMap[K mapBenchmarkKeyType, E mapBenchmarkElemType]() map[K]E {
	k := genValues[K](0, 700)
	e := genValues[E](0, 700)
	return fillMap(k, e)
}

var v1 int32
var v2 int64
var v3 string
var v4 smallType
var vok bool

//go:noinline
func call() {}

func BenchmarkUseAllVariants(b *testing.B) {
	m1 := makeMap[int32, int32]()
	m2 := makeMap[int64, int64]()
	m3 := makeMap[string, string]()
	m4 := makeMap[smallType, smallType]()
	var k1 int32
	var k2 int64
	var k3 string
	var k4 smallType
	for range b.N {
		call()
		v1 = m1[k1]
		call()
		v1, vok = m1[k1]

		call()
		v2 = m2[k2]
		call()
		v2, vok = m2[k2]

		call()
		v3 = m3[k3]
		call()
		v3, vok = m3[k3]
		call()

		call()
		v4 = m4[k4]
		call()
		v4, vok = m4[k4]
	}
}

goos: linux
goarch: amd64
pkg: runtime
cpu: AMD Ryzen 5 4600G with Radeon Graphics
                  │ /tmp/before3 │            /tmp/after3             │
                  │    sec/op    │   sec/op     vs base               │
UseAllVariants-12    69.51n ± 2%   63.68n ± 2%  -8.39% (p=0.000 n=30)

Before is the code above, after is every mapassign replaced with mapassign2 (v, vok = m[k]).
Shows a 8% improvement in performance. (also note that the faster version does more work (4 more stores to the vok global)).

This benchamrk shows that reducing the amount of variants might even speed up the overall performance. The benchmark above is a worst-case, as it uses 4 map variants at the same time.

[prattmic]

cc @golang/runtime

[prattmic]

If we do this, I think we'd want the latter option of using mapaccess2 everywhere, to reduce code size on the caller size. I agree that I think the only difference in mapaccess2 should be moving 1 to the ok register, which probably won't matter.

Before register ABI, callers would need additional stack space for the return value. I don't think that is a problem with the register ABI. I don't think we'll generate a spill slot on the stack for the dead variable.

This would certainly be a nice simplification, though I'm quite skeptical of the 8% reduction you mentioned.

@randall77 Am I forgetting an obvious reason we should keep both?

[randall77]

I think it is fine to just keep one (the *2 version). With the register ABI it's pretty cheap to return an extra constant.