Replace golang.org/x/exp/maps with stdlib maps

[Juneezee]

What changed?

The difference is that maps.Keys/maps.Values in the golang.org/x/exp/maps package return a slice, whereas maps.Keys/maps.Values in the standard library return an iterator. To work with slices, we need to use slices.Collect to convert the iterator into a slice.

Reference: https://go.dev/doc/go1.23#iterators

Why?

Part of the effort for #6366

How did you test it?

make unit-test

Potential risks

Documentation

Is hotfix candidate?

No

[dnr]

In an internal conversation, someone pointed out that iteration using maps.Values was significantly slower than for loop iteration (in go 1.23; 1.24 might be better). Many of these changes aren't critical but some may be in fairly hot paths. It would be nice to have some evidence this won't cause any performance decrease.

[Juneezee]

I wrote a quick benchmark:

package main_test

import (
	"fmt"
	stdmaps "maps"
	"slices"
	"testing"

	expmaps "golang.org/x/exp/maps"
)

func BenchmarkMapsKeys(b *testing.B) {
	// Populate map with test data
	size := 1000
	m := make(map[string]struct{}, size)
	for i := 0; i < size; i++ {
		m[fmt.Sprintf("key-%d", i)] = struct{}{}
	}
	b.ResetTimer()

	b.Run("stdmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			keys := slices.Collect(stdmaps.Keys(m))
			if len(keys) == 0 {
				b.Fail()
			}
		}
	})

	b.Run("expmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			keys := expmaps.Keys(m)
			if len(keys) == 0 {
				b.Fail()
			}
		}
	})
}

func BenchmarkMapsValues(b *testing.B) {
	// Populate map with test data
	size := 1000
	m := make(map[string]string, size)
	for i := 0; i < size; i++ {
		s := fmt.Sprintf("key-%d", i)
		m[s] = s
	}
	b.ResetTimer()

	b.Run("stdmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			vals := slices.Collect(stdmaps.Values(m))
			if len(vals) == 0 {
				b.Fail()
			}
		}
	})

	b.Run("expmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			vals := expmaps.Values(m)
			if len(vals) == 0 {
				b.Fail()
			}
		}
	})
}

Result:

goos: linux
goarch: amd64
pkg: go.temporal.io/server
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMapsKeys
BenchmarkMapsKeys/stdmaps
BenchmarkMapsKeys/stdmaps-16         	   27235	     44416 ns/op	   35184 B/op	      11 allocs/op
BenchmarkMapsKeys/expmaps
BenchmarkMapsKeys/expmaps-16         	   33115	     36327 ns/op	   16384 B/op	       1 allocs/op
BenchmarkMapsValues
BenchmarkMapsValues/stdmaps
BenchmarkMapsValues/stdmaps-16       	   25857	     45874 ns/op	   35184 B/op	      11 allocs/op
BenchmarkMapsValues/expmaps
BenchmarkMapsValues/expmaps-16       	   31256	     38348 ns/op	   16384 B/op	       1 allocs/op
PASS
coverage: [no statements]
ok  	go.temporal.io/server	6.489s

[dnr]

That's kind of unfortunate. Was that with go 1.23 or 1.24? I wonder if 1.24 is better? (I saw an issue where someone said there were more optimizations coming for iterators)

Maybe we should wait on this until the performance (and especially allocation) is more on par? What's the overall goal here, anyway?

[Juneezee]

That was with Go 1.24.0.

The result below is from Go 1.23.6.

~/Desktop/github/temporal ❯ go version
go version go1.23.6 linux/amd64

~/Desktop/github/temporal refactor/exp ❯ go test -v -bench . -benchmem ./main_test.go
goos: linux
goarch: amd64
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMapsKeys
BenchmarkMapsKeys/stdmaps
BenchmarkMapsKeys/stdmaps-16         	   21732	     54697 ns/op	   35248 B/op	      14 allocs/op
BenchmarkMapsKeys/expmaps
BenchmarkMapsKeys/expmaps-16         	   29280	     41233 ns/op	   16384 B/op	       1 allocs/op
BenchmarkMapsValues
BenchmarkMapsValues/stdmaps
BenchmarkMapsValues/stdmaps-16       	   20748	     56257 ns/op	   35248 B/op	      14 allocs/op
BenchmarkMapsValues/expmaps
BenchmarkMapsValues/expmaps-16       	   30194	     40220 ns/op	   16384 B/op	       1 allocs/op
PASS
ok  	command-line-arguments	6.757s

[Juneezee]

I think the performance loss is because slices.Collect does not preallocate the slice:

https://github.com/golang/go/blob/go1.24.0/src/slices/iter.go#L56-L59

while expmaps.Keys and expmaps.Values preallocate the slice:

https://github.com/golang/exp/blob/master/maps/maps.go#L10-L40

If we use slices.AppendSeq with preallocation instead of slices.Collect, then the results are the same:

package main_test

import (
	"fmt"
	stdmaps "maps"
	"slices"
	"testing"

	expmaps "golang.org/x/exp/maps"
)

func BenchmarkMapsKeys(b *testing.B) {
	// Populate map with test data
	size := 1000
	m := make(map[string]struct{}, size)
	for i := 0; i < size; i++ {
		m[fmt.Sprintf("key-%d", i)] = struct{}{}
	}
	b.ResetTimer()

	b.Run("stdmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			keys := slices.AppendSeq(make([]string, 0, len(m)), stdmaps.Keys(m))
			if len(keys) == 0 {
				b.Fail()
			}
		}
	})

	b.Run("expmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			keys := expmaps.Keys(m)
			if len(keys) == 0 {
				b.Fail()
			}
		}
	})
}

func BenchmarkMapsValues(b *testing.B) {
	// Populate map with test data
	size := 1000
	m := make(map[string]string, size)
	for i := 0; i < size; i++ {
		s := fmt.Sprintf("key-%d", i)
		m[s] = s
	}
	b.ResetTimer()

	b.Run("stdmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			vals := slices.AppendSeq(make([]string, 0, len(m)), stdmaps.Values(m))
			if len(vals) == 0 {
				b.Fail()
			}
		}
	})

	b.Run("expmaps", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			vals := expmaps.Values(m)
			if len(vals) == 0 {
				b.Fail()
			}
		}
	})
}

Result:

~/Desktop/github/temporal refactor/exp ❯ go version
go version go1.24.0 linux/amd64

~/Desktop/github/temporal refactor/exp ❯ go test -v -bench . -benchmem ./main_test.go
goos: linux
goarch: amd64
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkMapsKeys
BenchmarkMapsKeys/stdmaps
BenchmarkMapsKeys/stdmaps-16         	   34324	     35463 ns/op	   16384 B/op	       1 allocs/op
BenchmarkMapsKeys/expmaps
BenchmarkMapsKeys/expmaps-16         	   33741	     35582 ns/op	   16384 B/op	       1 allocs/op
BenchmarkMapsValues
BenchmarkMapsValues/stdmaps
BenchmarkMapsValues/stdmaps-16       	   33404	     35851 ns/op	   16384 B/op	       1 allocs/op
BenchmarkMapsValues/expmaps
BenchmarkMapsValues/expmaps-16       	   33709	     35585 ns/op	   16384 B/op	       1 allocs/op
PASS
ok  	command-line-arguments	6.270s

[Juneezee]

FYI, the proposal to make maps.Keys and maps.Values return an iterator instead of a slice is discussed in golang/go#61538.