[API] TraceState::Set overwrites an entry with the same key - #4586
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4586 +/- ##
==========================================
+ Coverage 86.51% 86.52% +0.01%
==========================================
Files 525 525
Lines 20464 20475 +11
==========================================
+ Hits 17702 17713 +11
Misses 2762 2762
🚀 New features to boost your workflow:
|
| } | ||
| auto allocate_size = curr_size; | ||
| if (curr_size < kMaxKeyValuePairs) | ||
| const size_t curr_size = kv_properties_->Size(); |
There was a problem hiding this comment.
This change to Set is an observable behavior change but also a correctness change. Before if a key passed to Set was an existing key for an entry then a new entry would be added with the same key. This violates the spec that keys must be unique.
| { | ||
| allocate_size -= 1; | ||
| } | ||
| const size_t curr_size = kv_properties_->Size(); |
There was a problem hiding this comment.
This change to delete is not an observable behavior change. The intent is to improve performance by replacing the GetValue call with HasKey to prevent unnecessary value copies.
…invalid and must be dropped
Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
marcalff
left a comment
There was a problem hiding this comment.
LGTM, see a minor optimization comment.
Fixes #4583
Changes
KeyValueProperties::HasKeymethod to prevent unneeded value copiesTraceState::Setto overwrite an existing entry if the key matchesTraceState::Deleteto remove unnecessary kv value copies.TraceState::FromHeaderto discard duplicate keys.For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes