Skip to content

Commit

Permalink
executor: adapt to the go1.21 map loadFactor setting value (#50545) (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
zhexuany committed Jan 25, 2024
1 parent 83df900 commit 44efbde
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 49 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ require (
github.com/gorilla/mux v1.8.0
github.com/gostaticanalysis/forcetypeassert v0.1.0
github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
github.com/hashicorp/go-version v1.6.0
github.com/iancoleman/strcase v0.2.0
github.com/jedib0t/go-pretty/v6 v6.2.2
github.com/jellydator/ttlcache/v3 v3.0.1
Expand Down
1 change: 1 addition & 0 deletions pkg/executor/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ go_test(
"//pkg/util/tableutil",
"//pkg/util/topsql/state",
"@com_github_gorilla_mux//:mux",
"@com_github_hashicorp_go_version//:go-version",
"@com_github_pingcap_errors//:errors",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_pingcap_fn//:fn",
Expand Down
2 changes: 1 addition & 1 deletion pkg/executor/concurrent_map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestConcurrentMap(t *testing.T) {

func TestConcurrentMapMemoryUsage(t *testing.T) {
m := newConcurrentMap()
const iterations = 1024 * hack.LoadFactorNum / hack.LoadFactorDen
var iterations = 1024 * hack.LoadFactorNum / hack.LoadFactorDen
var memUsage int64
wg := &sync.WaitGroup{}
wg.Add(2)
Expand Down
146 changes: 102 additions & 44 deletions pkg/executor/executor_pkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import (
"context"
"runtime"
"strconv"
"strings"
"testing"
"time"
"unsafe"

"github.com/hashicorp/go-version"
"github.com/pingcap/failpoint"
"github.com/pingcap/tidb/pkg/executor/aggfuncs"
"github.com/pingcap/tidb/pkg/executor/aggregate"
Expand Down Expand Up @@ -164,57 +166,113 @@ func TestSlowQueryRuntimeStats(t *testing.T) {
}

// Test whether the actual buckets in Golang Map is same with the estimated number.
// The test relies the implement of Golang Map. ref https://github.com/golang/go/blob/go1.13/src/runtime/map.go#L114
// The test relies on the implement of Golang Map. ref https://github.com/golang/go/blob/go1.13/src/runtime/map.go#L114
func TestAggPartialResultMapperB(t *testing.T) {
if runtime.Version() < `go1.13` {
t.Skip("Unsupported version")
// skip err, since we guarantee the success of execution
go113, _ := version.NewVersion(`1.13`)
// go version format is `gox.y.z foobar`, we only need x.y.z part
// The following is pretty hacky, but it only in test which is ok to do so.
actualVer, err := version.NewVersion(runtime.Version()[2:6])
if err != nil {
t.Fatalf("Cannot get actual go version with error %v\n", err)
}
if actualVer.LessThan(go113) {
t.Fatalf("Unsupported version and should never use any version less than go1.13\n")
}
type testCase struct {
rowNum int
expectedB int
expectedGrowing bool
}
cases := []testCase{
{
rowNum: 0,
expectedB: 0,
expectedGrowing: false,
},
{
rowNum: 100,
expectedB: 5,
expectedGrowing: true,
},
{
rowNum: 10000,
expectedB: 11,
expectedGrowing: false,
},
{
rowNum: 1000000,
expectedB: 18,
expectedGrowing: false,
},
{
rowNum: 851968, // 6.5 * (1 << 17)
expectedB: 18,
expectedGrowing: true,
},
{
rowNum: 851969, // 6.5 * (1 << 17) + 1
expectedB: 18,
expectedGrowing: true,
},
{
rowNum: 425984, // 6.5 * (1 << 16)
expectedB: 17,
expectedGrowing: true,
},
{
rowNum: 425985, // 6.5 * (1 << 16) + 1
expectedB: 17,
expectedGrowing: true,
},
var cases []testCase
// https://github.com/golang/go/issues/63438
// in 1.21, the load factor of map is 6 rather than 6.5 and the go team refused to backport to 1.21.
if strings.Contains(runtime.Version(), `go1.21`) {
cases = []testCase{
{
rowNum: 0,
expectedB: 0,
expectedGrowing: false,
},
{
rowNum: 95,
expectedB: 4,
expectedGrowing: false,
},
{
rowNum: 10000, // 6 * (1 << 11) is 12288
expectedB: 11,
expectedGrowing: false,
},
{
rowNum: 1000000, // 6 * (1 << 18) is 1572864
expectedB: 18,
expectedGrowing: false,
},
{
rowNum: 786432, // 6 * (1 << 17)
expectedB: 17,
expectedGrowing: false,
},
{
rowNum: 786433, // 6 * (1 << 17) + 1
expectedB: 18,
expectedGrowing: true,
},
{
rowNum: 393216, // 6 * (1 << 16)
expectedB: 16,
expectedGrowing: false,
},
{
rowNum: 393217, // 6 * (1 << 16) + 1
expectedB: 17,
expectedGrowing: true,
},
}
} else {
cases = []testCase{
{
rowNum: 0,
expectedB: 0,
expectedGrowing: false,
},
{
rowNum: 100,
expectedB: 4,
expectedGrowing: false,
},
{
rowNum: 10000,
expectedB: 11,
expectedGrowing: false,
},
{
rowNum: 1000000,
expectedB: 18,
expectedGrowing: false,
},
{
rowNum: 851968, // 6.5 * (1 << 17)
expectedB: 17,
expectedGrowing: false,
},
{
rowNum: 851969, // 6.5 * (1 << 17) + 1
expectedB: 18,
expectedGrowing: true,
},
{
rowNum: 425984, // 6.5 * (1 << 16)
expectedB: 16,
expectedGrowing: false,
},
{
rowNum: 425985, // 6.5 * (1 << 16) + 1
expectedB: 17,
expectedGrowing: true,
},
}
}

for _, tc := range cases {
Expand Down
2 changes: 1 addition & 1 deletion pkg/executor/hash_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func testHashRowContainer(t *testing.T, hashFunc func() hash.Hash64, spill bool)

func TestConcurrentMapHashTableMemoryUsage(t *testing.T) {
m := newConcurrentMapHashTable()
const iterations = 1024 * hack.LoadFactorNum / hack.LoadFactorDen // 6656
var iterations = 1024 * hack.LoadFactorNum / hack.LoadFactorDen // 6656
wg := &sync.WaitGroup{}
wg.Add(2)
// Note: Now concurrentMapHashTable doesn't support inserting in parallel.
Expand Down
15 changes: 13 additions & 2 deletions pkg/util/hack/hack.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package hack

import (
"runtime"
"strings"
"unsafe"
)

Expand All @@ -41,12 +43,21 @@ func Slice(s string) []byte {
// Represent as LoadFactorNum/LoadFactorDen, to allow integer math.
// They are from the golang definition. ref: https://github.com/golang/go/blob/go1.13.15/src/runtime/map.go#L68-L71
const (
// LoadFactorNum is the numerator of load factor
LoadFactorNum = 13
// LoadFactorDen is the denominator of load factor
LoadFactorDen = 2
)

// LoadFactorNum is the numerator of load factor
var LoadFactorNum = 13

func init() {
// In go1.21, the load factor num becomes 12 and go team has decided not to backport the fix to 1.21.
// See more details in https://github.com/golang/go/issues/63438
if strings.Contains(runtime.Version(), `go1.21`) {
LoadFactorNum = 12
}
}

const (
// DefBucketMemoryUsageForMapStrToSlice = bucketSize*(1+unsafe.Sizeof(string) + unsafe.Sizeof(slice))+2*ptrSize
// ref https://github.com/golang/go/blob/go1.15.6/src/reflect/type.go#L2162.
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/set/mem_aware_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func EstimateMapSize(length int, bucketSize uint64) uint64 {
if length == 0 {
return 0
}
bInMap := uint64(math.Ceil(math.Log2(float64(length) * hack.LoadFactorDen / hack.LoadFactorNum)))
bInMap := uint64(math.Ceil(math.Log2(float64(length) * hack.LoadFactorDen / float64(hack.LoadFactorNum))))
return bucketSize * uint64(1<<bInMap)
}

Expand Down

0 comments on commit 44efbde

Please sign in to comment.