[pdata] Migrate immutable primitive slices to mutable wrappers #5971
Conversation
|
We need a rebase on this. |
dmitryax
force-pushed
the
mutable-slices-2
branch
from
e475d18 to
7220a79
Compare
Codecov ReportBase: 91.98% // Head: 91.99% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5971 +/- ##
==========================================
+ Coverage 91.98% 91.99% +0.01%
==========================================
Files 213 213
Lines 13320 13358 +38
==========================================
+ Hits 12252 12289 +37
Misses 850 850
- Partials 218 219 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
dmitryax
force-pushed
the
mutable-slices-2
branch
7 times, most recently
from
3b978e2 to
92458f8
Compare
dmitryax
force-pushed
the
mutable-slices-2
branch
6 times, most recently
from
4aedd34 to
30c0388
Compare
dmitryax
force-pushed
the
mutable-slices-2
branch
from
ec1a2de to
43b105a
Compare
Change API for primitive slice wrappers to allow changing items as it appeared to be expensive to update that data as immutable wrappers: it requires making two copies to update a slice
dmitryax
force-pushed
the
mutable-slices-2
branch
from
43b105a to
99b9d97
Compare
| } | ||
|
|
||
| // FromRaw copies raw []${itemType} into the slice ${structName}. | ||
| func (ms ${structName}) FromRaw(val []${itemType}) { |
There was a problem hiding this comment.
@bogdandrutu do you think we should return ms here?
Otherwise current inline usages of pcommon.NewImmutable...Slice() becomes pretty clunky
There was a problem hiding this comment.
As for me, I'm not sure about that. I don't like returning it and I don't like how it complicates the usages
There was a problem hiding this comment.
Let me apply current state of this PR on contrib, and we'll see
There was a problem hiding this comment.
It looks pretty good open-telemetry/opentelemetry-collector-contrib#14016 I believe we should keep current version
Sounds good. I'll follow up on those |
This PR changes API for primitive slice wrappers to allow changing items directly since it appeared to be expensive to update that data as immutable wrappers: it required making two copies to update a slice.
This is a solution alternative to #5947
Update #5859
The wrappers for primitive slices doesn't represent atomic objects, they are treated the same way as other "complex" collection wrapper e.g.
pcommon.Sliceorpcommon.Map