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.
Just an interesting diversion this morning, I remembered that we do have quite a few outstanding
PoolVector
bugs, and wondered how easy it would be to just makePoolVector
a wrapper aroundVector
(as they both offer COW).Some
PoolVector
bugs e.g.:#69804
#51852
#53665
#37154
Turns out, for a basic version, it wasn't super tricky. So I'm making a draft here just for reference in future should be decide to go down that route.
There's probably quite a few flaws here as it's quick and dirty. In particular the writes are pretty inefficient because it will call the COW stuff each element is accessed (and the whole point of the
Write
is to avoid this), and we'd want them to reference the COW data to prevent de-allocation and dangling pointers (which the old version also suffers from). But it's a proof of concept.Notes
PoolVector
bugs, but otoh it is as likely that this would introduce regressions as much as solving existing bugs 😁 , which is why I'm just adding this as a draft rather than going full steam on it.PoolVector
altogether and replaced cases byVector
. That's probably something we could do in places in 3.x, but we'd need aPoolVector
wrapper for backward compatibility I suspect, especially with bound languages.