From 430f7347915993a5543bfb00858ac337274528ba Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Thu, 26 Dec 2024 18:28:49 +0000 Subject: [PATCH] Add MD.Clone Signed-off-by: Jin Dong --- metadata.go | 28 ++++++++++++++ metadata_test.go | 99 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/metadata.go b/metadata.go index ce8c0d1..6e00424 100644 --- a/metadata.go +++ b/metadata.go @@ -62,6 +62,34 @@ func (m MD) Append(key string, values ...string) { } } +// Clone returns a copy of MD or nil if it's nil. +// It's copied from golang's `http.Header.Clone` implementation: +// https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/net/http/header.go;l=94 +func (m MD) Clone() MD { + if m == nil { + return nil + } + + // Find total number of values. + nv := 0 + for _, vv := range m { + nv += len(vv) + } + sv := make([]string, nv) // shared backing array for headers' values + m2 := make(MD, len(m)) + for k, vv := range m { + if vv == nil { + // Preserve nil values. + m2[k] = nil + continue + } + n := copy(sv, vv) + m2[k] = sv[:n:n] + sv = sv[n:] + } + return m2 +} + func (m MD) setRequest(r *Request) { for k, values := range m { for _, v := range values { diff --git a/metadata_test.go b/metadata_test.go index d7fc095..7793251 100644 --- a/metadata_test.go +++ b/metadata_test.go @@ -18,6 +18,8 @@ package ttrpc import ( "context" + "fmt" + "sync" "testing" ) @@ -106,3 +108,100 @@ func TestMetadataContext(t *testing.T) { t.Errorf("invalid metadata value: %q", bar) } } + +func TestMetadataClone(t *testing.T) { + var metadata MD + m2 := metadata.Clone() + if m2 != nil { + t.Error("MD.Clone() on nil metadata should return nil") + } + + metadata = MD{"nil": nil, "foo": {"bar"}, "baz": {"qux", "quxx"}} + m2 = metadata.Clone() + + if len(metadata) != len(m2) { + t.Errorf("unexpected number of keys: %d, expected: %d", len(m2), len(metadata)) + } + + for k, v := range metadata { + v2, ok := m2[k] + if !ok { + t.Errorf("key not found: %s", k) + } + if v == nil && v2 == nil { + continue + } + if v == nil || v2 == nil { + t.Errorf("unexpected nil value: %v, expected: %v", v2, v) + } + if len(v) != len(v2) { + t.Errorf("unexpected number of values: %d, expected: %d", len(v2), len(v)) + } + for i := range v { + if v[i] != v2[i] { + t.Errorf("unexpected value: %s, expected: %s", v2[i], v[i]) + } + } + } +} + +func TestMetadataCloneConcurrent(t *testing.T) { + metadata := make(MD) + metadata.Set("foo", "bar") + + var wg sync.WaitGroup + for i := 0; i < 20; i++ { + wg.Add(1) + go func() { + defer wg.Done() + m2 := metadata.Clone() + m2.Set("foo", "baz") + }() + } + wg.Wait() + // Concurrent modification should clone the metadata first to avoid panic + // due to concurrent map writes. + if val, ok := metadata.Get("foo"); !ok { + t.Error("metadata not found") + } else if val[0] != "bar" { + t.Errorf("invalid metadata value: %q", val[0]) + } +} + +func simpleClone(src MD) MD { + md := MD{} + for k, v := range src { + md[k] = append(md[k], v...) + } + return md +} + +func BenchmarkMetadataClone(b *testing.B) { + for _, sz := range []int{5, 10, 20, 50} { + b.Run(fmt.Sprintf("size=%d", sz), func(b *testing.B) { + metadata := make(MD) + for i := 0; i < sz; i++ { + metadata.Set("foo"+fmt.Sprint(i), "bar"+fmt.Sprint(i)) + } + + for i := 0; i < b.N; i++ { + _ = metadata.Clone() + } + }) + } +} + +func BenchmarkSimpleMetadataClone(b *testing.B) { + for _, sz := range []int{5, 10, 20, 50} { + b.Run(fmt.Sprintf("size=%d", sz), func(b *testing.B) { + metadata := make(MD) + for i := 0; i < sz; i++ { + metadata.Set("foo"+fmt.Sprint(i), "bar"+fmt.Sprint(i)) + } + + for i := 0; i < b.N; i++ { + _ = simpleClone(metadata) + } + }) + } +}