-
Notifications
You must be signed in to change notification settings - Fork 4.8k
metadata: replace added [][]string with O(1) delta linked list #9129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
notandruu
wants to merge
5
commits into
grpc:master
from
notandruu:metadata-outgoing-context-linked-list
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
a9619fa
metadata: replace added [][]string with O(1) delta linked list
notandruu 46a0ff5
fix nil iter.Seq2 panic: return emptySeq2 instead of nil
notandruu f661b4a
metadata: rename emptySeq2 yield param to _ to satisfy revive lint
notandruu 8c1b585
metadata: address review feedback on benchmarks and map allocation
notandruu 7ebc74e
metadata: drain lazy iterator in FromOutgoingContextRaw benchmark
notandruu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ package metadata // import "google.golang.org/grpc/metadata" | |
| import ( | ||
| "context" | ||
| "fmt" | ||
| "iter" | ||
| "strings" | ||
|
|
||
| "google.golang.org/grpc/internal" | ||
|
|
@@ -181,14 +182,15 @@ func AppendToOutgoingContext(ctx context.Context, kv ...string) context.Context | |
| panic(fmt.Sprintf("metadata: AppendToOutgoingContext got an odd number of input pairs for metadata: %d", len(kv))) | ||
| } | ||
| md, _ := ctx.Value(mdOutgoingKey{}).(rawMD) | ||
| added := make([][]string, len(md.added)+1) | ||
| copy(added, md.added) | ||
| kvCopy := make([]string, 0, len(kv)) | ||
| kvCopy := make([]string, len(kv)) | ||
| for i := 0; i < len(kv); i += 2 { | ||
| kvCopy = append(kvCopy, strings.ToLower(kv[i]), kv[i+1]) | ||
| kvCopy[i] = strings.ToLower(kv[i]) | ||
| kvCopy[i+1] = kv[i+1] | ||
| } | ||
| added[len(added)-1] = kvCopy | ||
| return context.WithValue(ctx, mdOutgoingKey{}, rawMD{md: md.md, added: added}) | ||
| return context.WithValue(ctx, mdOutgoingKey{}, rawMD{ | ||
| md: md.md, | ||
| added: &deltaKV{kv: kvCopy, prev: md.added}, | ||
| }) | ||
| } | ||
|
|
||
| // FromIncomingContext returns the incoming metadata in ctx if it exists. | ||
|
|
@@ -241,19 +243,44 @@ func copyOf(v []string) []string { | |
|
|
||
| // fromOutgoingContextRaw returns the un-merged, intermediary contents of rawMD. | ||
| // | ||
| // Remember to perform strings.ToLower on the keys, for both the returned MD (MD | ||
| // is a map, there's no guarantee it's created using our helper functions) and | ||
| // the extra kv pairs (AppendToOutgoingContext doesn't turn them into | ||
| // lowercase). | ||
| func fromOutgoingContextRaw(ctx context.Context) (MD, [][]string, bool) { | ||
| // Remember to perform strings.ToLower on the keys in the returned MD (MD is a | ||
| // map, there's no guarantee it's created using our helper functions). | ||
| // Keys yielded by the iterator are already lowercase as AppendToOutgoingContext | ||
| // normalizes them. | ||
| func fromOutgoingContextRaw(ctx context.Context) (MD, iter.Seq2[string, string], bool) { | ||
| raw, ok := ctx.Value(mdOutgoingKey{}).(rawMD) | ||
| if !ok { | ||
| return nil, nil, false | ||
| return nil, emptySeq2, false | ||
| } | ||
|
|
||
| return raw.md, raw.added, true | ||
| if raw.added == nil { | ||
| return raw.md, emptySeq2, true | ||
| } | ||
| // Count nodes to pre-allocate the reversal slice. | ||
| n := 0 | ||
| for d := raw.added; d != nil; d = d.prev { | ||
| n++ | ||
| } | ||
| // Collect newest-first; the iterator yields oldest-first (FIFO order). | ||
| nodes := make([]*deltaKV, 0, n) | ||
| for d := raw.added; d != nil; d = d.prev { | ||
| nodes = append(nodes, d) | ||
| } | ||
| return raw.md, func(yield func(string, string) bool) { | ||
| for i := len(nodes) - 1; i >= 0; i-- { | ||
| kv := nodes[i].kv | ||
| for j := 0; j+1 < len(kv); j += 2 { | ||
| if !yield(kv[j], kv[j+1]) { | ||
| return | ||
| } | ||
| } | ||
| } | ||
| }, true | ||
| } | ||
|
|
||
| // emptySeq2 is a no-op iterator returned when there are no appended key-value | ||
| // pairs, avoiding nil-function panics at call sites that unconditionally range. | ||
| var emptySeq2 iter.Seq2[string, string] = func(_ func(string, string) bool) {} | ||
|
|
||
| // FromOutgoingContext returns the outgoing metadata in ctx if it exists. | ||
| // | ||
| // All keys in the returned MD are lowercase. | ||
|
|
@@ -263,33 +290,42 @@ func FromOutgoingContext(ctx context.Context) (MD, bool) { | |
| return nil, false | ||
| } | ||
|
|
||
| mdSize := len(raw.md) | ||
| for i := range raw.added { | ||
| mdSize += len(raw.added[i]) / 2 | ||
| } | ||
|
|
||
| out := make(MD, mdSize) | ||
| out := make(MD, len(raw.md)) | ||
| for k, v := range raw.md { | ||
| // We need to manually convert all keys to lower case, because MD is a | ||
| // map, and there's no guarantee that the MD attached to the context is | ||
| // created using our helper functions. | ||
| key := strings.ToLower(k) | ||
| out[key] = copyOf(v) | ||
| } | ||
| for _, added := range raw.added { | ||
| if len(added)%2 == 1 { | ||
| panic(fmt.Sprintf("metadata: FromOutgoingContext got an odd number of input pairs for metadata: %d", len(added))) | ||
| } | ||
|
|
||
| for i := 0; i < len(added); i += 2 { | ||
| key := strings.ToLower(added[i]) | ||
| out[key] = append(out[key], added[i+1]) | ||
| // Merge appended kv pairs in FIFO order. Collect nodes newest-first, | ||
| // then replay oldest-first so later appends override earlier ones. | ||
| n := 0 | ||
| for d := raw.added; d != nil; d = d.prev { | ||
| n++ | ||
| } | ||
| nodes := make([]*deltaKV, 0, n) | ||
|
Comment on lines
+303
to
+307
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use a stack allocated array here also to avoid a heap alloc and an extra pass over the linked-list: var buf [16]*deltaKV
nodes := buf[:0]
for d := raw.added; d != nil; d = d.prev {
nodes = append(nodes, d)
} |
||
| for d := raw.added; d != nil; d = d.prev { | ||
| nodes = append(nodes, d) | ||
| } | ||
| for i := len(nodes) - 1; i >= 0; i-- { | ||
| kv := nodes[i].kv | ||
| for j := 0; j < len(kv); j += 2 { | ||
| out[kv[j]] = append(out[kv[j]], kv[j+1]) | ||
| } | ||
| } | ||
| return out, ok | ||
| } | ||
|
|
||
| // deltaKV is a node in a singly-linked list of key-value slices appended | ||
| // via AppendToOutgoingContext. The list is newest-first: each node's prev | ||
| // field points to the older delta. Keys in kv are already lowercased. | ||
| type deltaKV struct { | ||
| kv []string | ||
| prev *deltaKV | ||
| } | ||
|
|
||
| type rawMD struct { | ||
| md MD | ||
| added [][]string | ||
| added *deltaKV | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gemini suggested an optimizations here that improve performance slightly for most real world cases where n < 16 :