From fefc805d4d2a08f20614455e9e54665c78318a87 Mon Sep 17 00:00:00 2001 From: Joway Date: Fri, 19 Apr 2024 15:04:15 +0800 Subject: [PATCH] perf: metainfo reduce mallocgc (#199) --- cloud/metainfo/http.go | 62 +++++++++++++---- cloud/metainfo/http_test.go | 2 +- cloud/metainfo/kv.go | 65 +++++++++--------- cloud/metainfo/kvstore.go | 55 +++++++++++++++ cloud/metainfo/kvstore_test.go | 66 ++++++++++++++++++ cloud/metainfo/utils.go | 118 +++++++++++++++++++++------------ cloud/metainfo/utils_test.go | 14 ++++ 7 files changed, 292 insertions(+), 90 deletions(-) create mode 100644 cloud/metainfo/kvstore.go create mode 100644 cloud/metainfo/kvstore_test.go diff --git a/cloud/metainfo/http.go b/cloud/metainfo/http.go index 8b10dc8e..5d90ac83 100644 --- a/cloud/metainfo/http.go +++ b/cloud/metainfo/http.go @@ -76,35 +76,73 @@ func FromHTTPHeader(ctx context.Context, h HTTPHeaderCarrier) context.Context { if ctx == nil || h == nil { return ctx } - - var m *mapView - if x := getNode(ctx); x != nil { - m = x.mapView() - } else { - m = newMapView() + nd := getNode(ctx) + if nd == nil || nd.size() == 0 { + return newCtxFromHTTPHeader(ctx, h) } + + // inherit from exist ctx node + persistent := newKVStore() + transient := newKVStore() + sliceToMap(nd.persistent, persistent) + sliceToMap(nd.transient, transient) + + // insert new kvs from http header h.Visit(func(k, v string) { if len(v) == 0 { return } - kk := strings.ToLower(k) ln := len(kk) + if ln > lenHPT && strings.HasPrefix(kk, HTTPPrefixTransient) { + kk = HTTPHeaderToCGIVariable(kk[lenHPT:]) + transient[kk] = v + } else if ln > lenHPP && strings.HasPrefix(kk, HTTPPrefixPersistent) { + kk = HTTPHeaderToCGIVariable(kk[lenHPP:]) + persistent[kk] = v + } + }) + + // return original ctx if no invalid key in http header + if (persistent.size() + transient.size()) == 0 { + return ctx + } + + // make new kvs + nd = newNodeFromMaps(persistent, transient, nil) + persistent.recycle() + transient.recycle() + ctx = withNode(ctx, nd) + return ctx +} +func newCtxFromHTTPHeader(ctx context.Context, h HTTPHeaderCarrier) context.Context { + nd := &node{ + persistent: make([]kv, 0, 16), // 32B * 16 = 512B + transient: make([]kv, 0, 16), + stale: []kv{}, + } + // insert new kvs from http header to node + h.Visit(func(k, v string) { + if len(v) == 0 { + return + } + kk := strings.ToLower(k) + ln := len(kk) if ln > lenHPT && strings.HasPrefix(kk, HTTPPrefixTransient) { kk = HTTPHeaderToCGIVariable(kk[lenHPT:]) - m.transient[kk] = v + nd.transient = append(nd.transient, kv{key: kk, val: v}) } else if ln > lenHPP && strings.HasPrefix(kk, HTTPPrefixPersistent) { kk = HTTPHeaderToCGIVariable(kk[lenHPP:]) - m.persistent[kk] = v + nd.persistent = append(nd.persistent, kv{key: kk, val: v}) } }) - if m.size() == 0 { - // TODO: remove this? + // return original ctx if no invalid key in http header + if nd.size() == 0 { return ctx } - return withNode(ctx, m.toNode()) + return withNode(ctx, nd) } // ToHTTPHeader writes all metainfo into the given HTTP header. diff --git a/cloud/metainfo/http_test.go b/cloud/metainfo/http_test.go index 27b63a8f..b5f824c7 100644 --- a/cloud/metainfo/http_test.go +++ b/cloud/metainfo/http_test.go @@ -87,7 +87,7 @@ func TestFromHTTPHeaderKeepPreviousData(t *testing.T) { c1 := metainfo.FromHTTPHeader(c0, metainfo.HTTPHeader(h)) assert(t, c0 != c1) vs := metainfo.GetAllValues(c1) - assert(t, len(vs) == 3) + assert(t, len(vs) == 3, len(vs)) assert(t, vs["tk"] == "tv" && vs["uk"] == "uv" && vs["XK"] == "xv") vs = metainfo.GetAllPersistentValues(c1) assert(t, len(vs) == 3) diff --git a/cloud/metainfo/kv.go b/cloud/metainfo/kv.go index 07abd66d..6694c900 100644 --- a/cloud/metainfo/kv.go +++ b/cloud/metainfo/kv.go @@ -14,7 +14,9 @@ package metainfo -import "context" +import ( + "context" +) type ctxKeyType struct{} @@ -25,6 +27,33 @@ type kv struct { val string } +func newNodeFromMaps(persistent, transient, stale kvstore) *node { + ps, ts, sz := persistent.size(), transient.size(), stale.size() + // make slices together to reduce malloc cost + kvs := make([]kv, ps+ts+sz) + nd := new(node) + nd.persistent = kvs[:ps] + nd.transient = kvs[ps : ps+ts] + nd.stale = kvs[ps+ts:] + + i := 0 + for k, v := range persistent { + nd.persistent[i].key, nd.persistent[i].val = k, v + i++ + } + i = 0 + for k, v := range transient { + nd.transient[i].key, nd.transient[i].val = k, v + i++ + } + i = 0 + for k, v := range stale { + nd.stale[i].key, nd.stale[i].val = k, v + i++ + } + return nd +} + type node struct { persistent []kv transient []kv @@ -122,40 +151,6 @@ func (n *node) delPersistent(k string) (r *node) { return n } -type mapView struct { - persistent map[string]string - transient map[string]string - stale map[string]string -} - -func newMapView() *mapView { - return &mapView{ - persistent: make(map[string]string), - transient: make(map[string]string), - stale: make(map[string]string), - } -} - -func (m *mapView) size() int { - return len(m.persistent) + len(m.transient) + len(m.stale) -} - -func (m *mapView) toNode() *node { - return &node{ - persistent: mapToSlice(m.persistent), - transient: mapToSlice(m.transient), - stale: mapToSlice(m.stale), - } -} - -func (n *node) mapView() *mapView { - return &mapView{ - persistent: sliceToMap(n.persistent), - transient: sliceToMap(n.transient), - stale: sliceToMap(n.stale), - } -} - func search(kvs []kv, key string) (idx int, ok bool) { for i := range kvs { if kvs[i].key == key { diff --git a/cloud/metainfo/kvstore.go b/cloud/metainfo/kvstore.go new file mode 100644 index 00000000..a414104f --- /dev/null +++ b/cloud/metainfo/kvstore.go @@ -0,0 +1,55 @@ +// Copyright 2023 ByteDance Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metainfo + +import "sync" + +type kvstore map[string]string + +var kvpool sync.Pool + +func newKVStore(size ...int) kvstore { + kvs := kvpool.Get() + if kvs == nil { + if len(size) > 0 { + return make(kvstore, size[0]) + } + return make(kvstore) + } + return kvs.(kvstore) +} + +func (store kvstore) size() int { + return len(store) +} + +func (store kvstore) recycle() { + /* + for k := range m { + delete(m, k) + } + ==> + LEAQ type.map[string]int(SB), AX + MOVQ AX, (SP) + MOVQ "".m(SB), AX + MOVQ AX, 8(SP) + PCDATA $1, $0 + CALL runtime.mapclear(SB) + */ + for key := range store { + delete(store, key) + } + kvpool.Put(store) +} diff --git a/cloud/metainfo/kvstore_test.go b/cloud/metainfo/kvstore_test.go new file mode 100644 index 00000000..40edb448 --- /dev/null +++ b/cloud/metainfo/kvstore_test.go @@ -0,0 +1,66 @@ +// Copyright 2023 ByteDance Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metainfo + +import ( + "fmt" + "testing" +) + +func TestKVStore(t *testing.T) { + store := newKVStore() + store["a"] = "a" + store["a"] = "b" + if store["a"] != "b" { + t.Fatal() + } + store.recycle() + if store["a"] == "b" { + t.Fatal() + } + store = newKVStore() + if store["a"] == "b" { + t.Fatal() + } +} + +func BenchmarkMap(b *testing.B) { + for keys := 1; keys <= 1000; keys *= 10 { + b.Run(fmt.Sprintf("keys=%d", keys), func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + m := make(map[string]string) + for idx := 0; idx < 1000; idx++ { + m[fmt.Sprintf("key-%d", idx)] = string('a' + byte(idx%26)) + } + } + }) + } +} + +func BenchmarkKVStore(b *testing.B) { + for keys := 1; keys <= 1000; keys *= 10 { + b.Run(fmt.Sprintf("keys=%d", keys), func(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + m := newKVStore() + for idx := 0; idx < 1000; idx++ { + m[fmt.Sprintf("key-%d", idx)] = string('a' + byte(idx%26)) + } + m.recycle() + } + }) + } +} diff --git a/cloud/metainfo/utils.go b/cloud/metainfo/utils.go index 7f2cfc56..d5ceeeb8 100644 --- a/cloud/metainfo/utils.go +++ b/cloud/metainfo/utils.go @@ -28,48 +28,93 @@ func HasMetaInfo(ctx context.Context) bool { // Only those keys with prefixes defined in this module would be used. // If the context has been carrying metanifo pairs, they will be merged as a basis. func SetMetaInfoFromMap(ctx context.Context, m map[string]string) context.Context { - if ctx == nil { - return nil + if ctx == nil || len(m) == 0 { + return ctx + } + + nd := getNode(ctx) + if nd == nil || nd.size() == 0 { + // fast path + return newCtxFromMap(ctx, m) + } + // inherit from node + mapSize := len(m) + persistent := newKVStore(mapSize) + transient := newKVStore(mapSize) + stale := newKVStore(mapSize) + sliceToMap(nd.persistent, persistent) + sliceToMap(nd.transient, transient) + sliceToMap(nd.stale, stale) + + // insert new kvs from m to node + for k, v := range m { + if len(k) == 0 || len(v) == 0 { + continue + } + switch { + case strings.HasPrefix(k, PrefixTransientUpstream): + if len(k) > lenPTU { // do not move this condition to the case statement to prevent a PTU matches PT + stale[k[lenPTU:]] = v + } + case strings.HasPrefix(k, PrefixTransient): + if len(k) > lenPT { + transient[k[lenPT:]] = v + } + case strings.HasPrefix(k, PrefixPersistent): + if len(k) > lenPP { + persistent[k[lenPP:]] = v + } + } } - if len(m) == 0 { + // return original ctx if no invalid key in map + if (persistent.size() + transient.size() + stale.size()) == 0 { return ctx } - var mv *mapView - if x := getNode(ctx); x != nil { - mv = x.mapView() - } else { - mv = newMapView() + // make new node, and transfer map to list + nd = newNodeFromMaps(persistent, transient, stale) + persistent.recycle() + transient.recycle() + stale.recycle() + return withNode(ctx, nd) +} + +func newCtxFromMap(ctx context.Context, m map[string]string) context.Context { + // make new node + mapSize := len(m) + nd := &node{ + persistent: make([]kv, 0, mapSize), + transient: make([]kv, 0, mapSize), + stale: make([]kv, 0, mapSize), } + // insert new kvs from m to node for k, v := range m { if len(k) == 0 || len(v) == 0 { continue } - switch { case strings.HasPrefix(k, PrefixTransientUpstream): if len(k) > lenPTU { // do not move this condition to the case statement to prevent a PTU matches PT - mv.stale[k[lenPTU:]] = v + nd.stale = append(nd.stale, kv{key: k[lenPTU:], val: v}) } case strings.HasPrefix(k, PrefixTransient): if len(k) > lenPT { - mv.transient[k[lenPT:]] = v + nd.transient = append(nd.transient, kv{key: k[lenPT:], val: v}) } case strings.HasPrefix(k, PrefixPersistent): if len(k) > lenPP { - mv.persistent[k[lenPP:]] = v + nd.persistent = append(nd.persistent, kv{key: k[lenPP:], val: v}) } } } - if mv.size() == 0 { - // TODO: remove this? + // return original ctx if no invalid key in map + if nd.size() == 0 { return ctx } - - return withNode(ctx, mv.toNode()) + return withNode(ctx, nd) } // SaveMetaInfoToMap set key-value pairs from ctx to m while filtering out transient-upstream data. @@ -78,36 +123,25 @@ func SaveMetaInfoToMap(ctx context.Context, m map[string]string) { return } ctx = TransferForward(ctx) - for k, v := range GetAllValues(ctx) { - m[PrefixTransient+k] = v - } - for k, v := range GetAllPersistentValues(ctx) { - m[PrefixPersistent+k] = v - } -} - -// sliceToMap converts a kv slice to map. If the slice is empty, an empty map will be returned instead of nil. -func sliceToMap(slice []kv) (m map[string]string) { - if size := len(slice); size == 0 { - m = make(map[string]string) - } else { - m = make(map[string]string, size) - } - for _, kv := range slice { - m[kv.key] = kv.val + if n := getNode(ctx); n != nil { + for _, kv := range n.stale { + m[PrefixTransient+kv.key] = kv.val + } + for _, kv := range n.transient { + m[PrefixTransient+kv.key] = kv.val + } + for _, kv := range n.persistent { + m[PrefixPersistent+kv.key] = kv.val + } } - return } -// mapToSlice converts a map to a kv slice. If the map is empty, the return value will be nil. -func mapToSlice(kvs map[string]string) (slice []kv) { - size := len(kvs) - if size == 0 { +// sliceToMap converts a kv slice to map. +func sliceToMap(slice []kv, kvs kvstore) { + if len(slice) == 0 { return } - slice = make([]kv, 0, size) - for k, v := range kvs { - slice = append(slice, kv{key: k, val: v}) + for _, kv := range slice { + kvs[kv.key] = kv.val } - return } diff --git a/cloud/metainfo/utils_test.go b/cloud/metainfo/utils_test.go index 053ed268..1982d6c1 100644 --- a/cloud/metainfo/utils_test.go +++ b/cloud/metainfo/utils_test.go @@ -16,6 +16,7 @@ package metainfo_test import ( "context" + "fmt" "testing" "github.com/bytedance/gopkg/cloud/metainfo" @@ -146,3 +147,16 @@ func TestSaveMetaInfoToMap(t *testing.T) { assert(t, m[metainfo.PrefixTransient+"b"] == "b2") assert(t, m[metainfo.PrefixPersistent+"b"] == "b3") } + +func BenchmarkSetMetaInfoFromMap(b *testing.B) { + ctx := metainfo.WithPersistentValue(context.Background(), "key", "val") + m := map[string]string{} + for i := 0; i < 32; i++ { + m[fmt.Sprintf("key-%d", i)] = fmt.Sprintf("val-%d", i) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = metainfo.SetMetaInfoFromMap(ctx, m) + } +}