-
Notifications
You must be signed in to change notification settings - Fork 420
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
Add Metadata collection #1683
Add Metadata collection #1683
Conversation
e1fd59e
to
e8ad94f
Compare
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.
This is a great start @gjcairo!
c782124
to
a422f61
Compare
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.
This is coming along nicely!
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.
This looks great overall. Really love the new expressivity for metadata here! Not sure if we should do it in this PR or a follow up but we should have benchmarks for adding/removing/iterating the Metadata
. I can already see some methods on the various RandomAccessColllection
/Sequence
types that are prone to allocate due to missing @inlinable
on them.
It might be good to start adding perf benchmarks right away with our new v2 code instead of after the fact. WDYT @glbrntt ?
Look at radar ;) |
8d287ad
to
cb0acc3
Compare
354a195
to
1c30ad2
Compare
1c30ad2
to
382579f
Compare
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.
Couple of nits, otherwise this looks great!
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.
This is really great, thanks @gjcairo!
Motivation
This PR adds a new Metadata collection for gRPC Core. This collection has the following requirements:
Modifications
Implemented a new
Metadata
collection that follows the above requirements.Result
Metadata collection is available.