Skip to content
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

Naïve _update strategy could be suboptimal. #444

Open
dabrahams opened this issue Dec 24, 2024 · 1 comment
Open

Naïve _update strategy could be suboptimal. #444

dabrahams opened this issue Dec 24, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@dabrahams
Copy link

The problem is that withUnsafeMutableBufferPointer forces a copy if the storage is not uniquely referenced. If you are going to then rewrite/move a significant number of elements, the original copies are wasted effort, which could be bad for locality. What you really want in those cases is to construct the new buffer in its final form in-situ. This is just something to consider/benchmark as you look to further optimize. Starting with the current strategy certainly makes sense.

@dabrahams dabrahams added the bug Something isn't working label Dec 24, 2024
@lorentey
Copy link
Member

Oh yes yes! Bulk mutations making a copy of items only to immediately throw most of them away has been frustrating me for years.

Ideally we'd have three code paths for most mutation operations:

storage is uniquely held it has enough room strategy
yes yes in-place mutation of existing storage
yes no populate new storage by moving items
no any populate new storage by copying items

(With a potential middle ground where the second strategy (moving items) is rolled into the third.)

Currently, mutation operations often have just one implementation: in-place mutation. The uniqueness/capacity preconditions are ensured up front in a uniform (but wasteful) way. This keeps the happy path fast, but it makes slow paths slower than necessary.

There are two excuses for why we do it like that:

  1. I worry about blowing up code size, especially as we expect these operations to get specialized for every type parameter
  2. I worry about introducing bugs, as I don't really have a good way to systematically test the newly introduced code paths

(Well, these two, and the fact that people haven't much complained about mutation performance for non-uniquely held collections...)

FWIW, I'm working on replacing the _read/_update pattern with a new library construct; this provides a good opportunity to refactor things along these lines!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants