Skip to content

[OrderedCollections] Add move operations#660

Merged
lorentey merged 4 commits into
mainfrom
dn/ordered-set-move
Jun 6, 2026
Merged

[OrderedCollections] Add move operations#660
lorentey merged 4 commits into
mainfrom
dn/ordered-set-move

Conversation

@dnadoba

@dnadoba dnadoba commented May 26, 2026

Copy link
Copy Markdown
Member

Description

Adds new method for relocating elements within an OrderedSet and OrderedDictionary:

extension OrderedSet {
  mutating func moveSubrange(_ range: some RangeExpression<Index>, to destination: Index)
  mutating func move(members: some Sequence<Element>, to destination: Index)
  mutating func move(indices: some Sequence<Index>, to destination: Index)
}
extension OrderedDictionary {
  mutating func moveSubrange(_ range: some RangeExpression<Index>, to destination: Index)
  mutating func move(keys: some Sequence<Key>, to destination: Index)
  mutating func move(indices: some Sequence<Index>, to destination: Index)
}

This fills a gap that comes up regularly in drag-and-drop UIs and other reordering workflows: today users have to fall back to remove(at:) + insert(_:at:), which is O(count * k) and re-hashes elements unnecessarily.

Performance

12 new benchmarks in OrderedSetBenchmarks cover the public API across the four dispatch paths. The charts below were produced by disabling the fast paths selectively.

Fast path 1: contiguous-range detection in move(_:Sequence). When the input forms a contiguous range, dispatching to the range-rotation path avoids the compact-and-place allocation.

fp1-contiguous-detection

Fast path 2: contiguous-reordered detection. When the sorted sources are contiguous but the input is in a different order, rotation followed by in-place permutation beats compact-and-place.

fp2-contiguous-reordered-detection

Fast path 3: targeted hash updates in move(_:Range). For small range moves over short distances, touching just the affected buckets beats scanning the whole hash table.

fp3-targeted-hash-range

Fast path 4: targeted hash updates in scattered moves. When the affected region is small relative to table capacity, per-element bucket updates beat a full hash-table rebuild.

fp4-targeted-hash-scattered

All benchmarks:
all-move-benchmarks

Source Impact

Additive.

Checklist

  • I've read the Contribution Guidelines
  • My contributions are licensed under the Swift license.
  • I've followed the coding style of the rest of the project.
  • I've added tests covering all new code paths my change adds to the project (to the extent possible).
  • I've added benchmarks covering new functionality (if appropriate).
  • I've verified that my change does not break any existing tests or introduce unexpected benchmark regressions.
  • I've updated the documentation (if appropriate).

@dnadoba dnadoba requested a review from lorentey as a code owner May 26, 2026 03:56
@dnadoba dnadoba force-pushed the dn/ordered-set-move branch from 4d3fd21 to 5e53ecc Compare May 26, 2026 03:59
Adds two overloads for relocating elements within an `OrderedSet`: one
taking a contiguous `Range<Int>`, and one taking a `Sequence<Element>`.
The moved elements occupy consecutive positions starting at the
destination, in the order they appear in the input.

The hot path runs in O(*d* + *k*) time, where *d* is the distance moved
and *k* is the number of elements moved; the implementation falls back
to O(`count`) for large moves.
@dnadoba dnadoba force-pushed the dn/ordered-set-move branch from 5e53ecc to 508f36f Compare May 26, 2026 06:03
@lorentey lorentey added this to the 1.6.0 milestone May 27, 2026

@lorentey lorentey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done! I have some naming recommendations and a refactoring suggestion

Comment thread Sources/OrderedCollections/OrderedSet/OrderedSet+Move.swift Outdated
Comment thread Sources/OrderedCollections/OrderedSet/OrderedSet+Move.swift
Comment thread Sources/OrderedCollections/OrderedSet/OrderedSet+Move.swift Outdated
Comment thread Sources/OrderedCollections/OrderedSet/OrderedSet+Move.swift Outdated
Reworks the relocation API and extends it to `OrderedDictionary`:

* Renames the index-range move to `moveSubrange(_:to:)` (with a
  `RangeExpression` overload) and the by-value move to `move(contentsOf:to:)`,
  matching the conventions of `removeSubrange`/`insert(_:at:)`. Adds
  `move(fromIndices:to:)`, which takes the source indices directly.
* Factors the element rearrangement into generic
  `UnsafeMutableBufferPointer` operations (rotation / rotation+permutation /
  scatter) so `OrderedDictionary` relocates its keys and values in lockstep
  through the same code, adding only the parallel value move; the hash-table
  maintenance stays entirely in `OrderedSet`.
* The hot path runs in O(*d* + *k*) and falls back to O(`count`) for large
  moves; bucket maintenance chooses targeted updates or a full rebuild by the
  same threshold.
* `move(contentsOf:to:)` ignores elements/keys that aren't present; duplicate
  members are detected and trap.

Adds tests (exhaustive hash-table integrity, value alignment under lifetime
tracking) and move benchmarks for both types.
@dnadoba

dnadoba commented Jun 5, 2026

Copy link
Copy Markdown
Member Author

Update public API:

extension OrderedSet {
  public mutating func moveSubrange(_ range: some RangeExpression<Index>, to destination: Index)
  public mutating func move(contentsOf elements: some Sequence<Element>, to destination: Index)
  public mutating func move(fromIndices indices: [Index], to destination: Index)
}

I also recalled an older conversation with my team and update move(contentsOf:) to ignore elements/keys that aren't present. That can sometimes happen during a drag & drop operation that races with a concurrent mutation from e.g. a network sync from other devices or other users.

I have also added another fast path. When a scattered move requires a full hash-table rebuild and the OrderedSet's table storage is not uniquely referenced, it builds a fresh table from scratch instead of copying the shared table only to immediately clear and refill it.

While refactoring it was now also trivial to add API to OrderedDictionary:

extension OrderedDictionary {
  public mutating func moveSubrange(_ range: some RangeExpression<Index>, to destination: Index)
  public mutating func move(contentsOf keys: some Sequence<Key>, to destination: Index)
  public mutating func move(fromIndices indices: [Index], to destination: Index)
}

OrderedSet resolves the move and calls back through an internal closure with the resolved offset views, and the dictionary applies the same rearrangement to its values in lockstep with the keys. It has to be a closure rather than returning the offsets because those offset buffers are stack-allocated via withUnsafeTemporaryAllocation and can't escape the call. It also has to be a non-optional closure as optional closures become escaping closures and then the use site can't capture the values inout.

I wasn't too sure about the fromIndices parameter label. Let me know if you have a better idea and I can rename it.

Comment thread Sources/OrderedCollections/OrderedSet/OrderedSet+Move.swift Outdated
extension OrderedSet {
@inlinable
internal mutating func _move(
fromIndices sourceOffsets: UnsafeBufferPointer<Int>,

@dnadoba dnadoba Jun 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I could also replace all (or most) UnsafeBufferPointer usage with Span internally.
I shortly experimented and it seemed that at least with Swift 6.4 I didn't run into any limitations and it seemed to work just fine.

Downside is that it will require at least Swift 6.2+, maybe higher depending on if we need any API that came out later.
What's your feeling here? I don't think anything else in this module uses Span so it might be a bit inconstant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine!

Span has SwiftStdlib 5.0 availability and that prevents us from migrating existing code to use it until the minimum deployment target of our oldest supported toolchain grows beyond that. (New API additions can start using it right now, though; they would just need to be marked with @available(SwiftStdlib 5.0, *).

@dnadoba dnadoba changed the title [OrderedSet] Add move(_:toOffset:) [OrderedCollections] Add move operations Jun 5, 2026
@dnadoba dnadoba force-pushed the dn/ordered-set-move branch from 7b322de to 5a6af29 Compare June 5, 2026 21:44
extension OrderedSet {
@inlinable
internal mutating func _move(
fromIndices sourceOffsets: UnsafeBufferPointer<Int>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine!

Span has SwiftStdlib 5.0 availability and that prevents us from migrating existing code to use it until the minimum deployment target of our oldest supported toolchain grows beyond that. (New API additions can start using it right now, though; they would just need to be marked with @available(SwiftStdlib 5.0, *).

@lorentey lorentey merged commit 5c8d97d into main Jun 6, 2026
65 of 68 checks passed
@lorentey lorentey deleted the dn/ordered-set-move branch June 6, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants