From 2e523b1045ca2b4d48cf7841a577057a1e7bd203 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:09:21 -0700 Subject: [PATCH 01/18] [PersistentCollections] Reverse ordering of items in node storage --- .../Node/_Node+CustomStringConvertible.swift | 2 +- .../Node/_Node+Debugging.swift | 19 +++++----- .../Node/_Node+Equatable.swift | 6 ++-- .../Node/_Node+Initializers.swift | 12 +++---- .../Node/_Node+Lookups.swift | 7 ++-- .../Node/_Node+Mutations.swift | 36 ++++++++++--------- .../Node/_Node+Storage.swift | 8 +++-- ...eHandle.swift => _Node+UnsafeHandle.swift} | 4 +-- .../PersistentCollections/Node/_Node.swift | 4 ++- .../PersistentDictionary+Sequence.swift | 6 ++-- 10 files changed, 55 insertions(+), 49 deletions(-) rename Sources/PersistentCollections/Node/{_Node.UnsafeHandle.swift => _Node+UnsafeHandle.swift} (97%) diff --git a/Sources/PersistentCollections/Node/_Node+CustomStringConvertible.swift b/Sources/PersistentCollections/Node/_Node+CustomStringConvertible.swift index becde41aa..dda52b375 100644 --- a/Sources/PersistentCollections/Node/_Node+CustomStringConvertible.swift +++ b/Sources/PersistentCollections/Node/_Node+CustomStringConvertible.swift @@ -19,7 +19,7 @@ extension _Node: CustomStringConvertible { var result = "[" var first = true read { - for (key, value) in $0._items { + for (key, value) in $0.reverseItems.reversed() { if first { first = false } else { diff --git a/Sources/PersistentCollections/Node/_Node+Debugging.swift b/Sources/PersistentCollections/Node/_Node+Debugging.swift index 1dda3cd0c..794439ec8 100644 --- a/Sources/PersistentCollections/Node/_Node+Debugging.swift +++ b/Sources/PersistentCollections/Node/_Node+Debugging.swift @@ -34,6 +34,12 @@ extension _Node.Storage { } extension _Node.UnsafeHandle { + internal func _itemString(at offset: Int) -> String { + let item = self[item: offset] + let hash = _Hash(item.key).description + return "hash: \(hash), key: \(item.key), value: \(item.value)" + } + @usableFromInline internal func dump( firstPrefix: String = "", @@ -50,12 +56,8 @@ extension _Node.UnsafeHandle { """) guard limit > 0 else { return } if isCollisionNode { - let items = self._items - for offset in items.indices { - let item = items[offset] - let hash = _Hash(item.key).description - let itemStr = "hash: \(hash), key: \(item.key), value: \(item.value)" - print("\(restPrefix) \(offset): \(itemStr)") + for offset in 0 ..< itemCount { + print("\(restPrefix)[\(offset)] \(_itemString(at: offset))") } } else { var itemOffset = 0 @@ -64,10 +66,7 @@ extension _Node.UnsafeHandle { let bucket = _Bucket(b) let bucketStr = "#\(String(b, radix: _Bitmap.capacity, uppercase: true))" if itemMap.contains(bucket) { - let item = self[item: itemOffset] - let hash = _Hash(item.key).description - let itemStr = "hash: \(hash), key: \(item.key), value: \(item.value)" - print("\(restPrefix) \(bucketStr) \(itemStr)") + print("\(restPrefix) \(bucketStr) \(_itemString(at: itemOffset))") itemOffset += 1 } else if childMap.contains(bucket) { self[child: childOffset].dump( diff --git a/Sources/PersistentCollections/Node/_Node+Equatable.swift b/Sources/PersistentCollections/Node/_Node+Equatable.swift index 5ec3f2d69..bd90fa0e9 100644 --- a/Sources/PersistentCollections/Node/_Node+Equatable.swift +++ b/Sources/PersistentCollections/Node/_Node+Equatable.swift @@ -18,9 +18,9 @@ extension _Node: Equatable where Value: Equatable { return lhs.read { lhs in rhs.read { rhs in let l = Dictionary( - uniqueKeysWithValues: lhs._items.lazy.map { ($0.key, $0.value) }) + uniqueKeysWithValues: lhs.reverseItems.lazy.map { ($0.key, $0.value) }) let r = Dictionary( - uniqueKeysWithValues: rhs._items.lazy.map { ($0.key, $0.value) }) + uniqueKeysWithValues: rhs.reverseItems.lazy.map { ($0.key, $0.value) }) return l == r } } @@ -39,7 +39,7 @@ extension _Node: Equatable where Value: Equatable { guard l.itemMap == r.itemMap else { return false } guard l.childMap == r.childMap else { return false } - guard l._items.elementsEqual(r._items, by: { $0 == $1 }) + guard l.reverseItems.elementsEqual(r.reverseItems, by: { $0 == $1 }) else { return false } guard l._children.elementsEqual(r._children) else { return false } diff --git a/Sources/PersistentCollections/Node/_Node+Initializers.swift b/Sources/PersistentCollections/Node/_Node+Initializers.swift index 2d89eddce..86ba091c2 100644 --- a/Sources/PersistentCollections/Node/_Node+Initializers.swift +++ b/Sources/PersistentCollections/Node/_Node+Initializers.swift @@ -20,9 +20,9 @@ extension _Node { let byteCount = 2 * MemoryLayout.stride assert($0.bytesFree >= byteCount) $0.bytesFree &-= byteCount - let items = $0._items - items.initializeElement(at: 0, to: item1) - items.initializeElement(at: 1, to: item2) + let items = $0.reverseItems + items.initializeElement(at: 1, to: item1) + items.initializeElement(at: 0, to: item2) } } @@ -39,10 +39,10 @@ extension _Node { $0.itemMap.insert(bucket1) $0.itemMap.insert(bucket2) $0.bytesFree &-= 2 * MemoryLayout.stride - let i = bucket1 < bucket2 ? 0 : 1 - let items = $0._items + let i = bucket1 < bucket2 ? 1 : 0 + let items = $0.reverseItems items.initializeElement(at: i, to: item1) - items.initializeElement(at: 1 - i, to: item2) + items.initializeElement(at: 1 &- i, to: item2) } } diff --git a/Sources/PersistentCollections/Node/_Node+Lookups.swift b/Sources/PersistentCollections/Node/_Node+Lookups.swift index 3c78bd3db..182f7d27d 100644 --- a/Sources/PersistentCollections/Node/_Node+Lookups.swift +++ b/Sources/PersistentCollections/Node/_Node+Lookups.swift @@ -94,10 +94,11 @@ extension _Node.UnsafeHandle { return .expansion(h) } } - guard let offset = _items.firstIndex(where: { $0.key == key }) else { - return .notFound(.invalid, 0) + // Note: this searches the items in reverse insertion order. + guard let offset = reverseItems.firstIndex(where: { $0.key == key }) else { + return .notFound(.invalid, itemCount) } - return .found(.invalid, offset) + return .found(.invalid, itemCount &- 1 &- offset) } let bucket = hash[level] if itemMap.contains(bucket) { diff --git a/Sources/PersistentCollections/Node/_Node+Mutations.swift b/Sources/PersistentCollections/Node/_Node+Mutations.swift index a4cd71d19..b23ee41ba 100644 --- a/Sources/PersistentCollections/Node/_Node+Mutations.swift +++ b/Sources/PersistentCollections/Node/_Node+Mutations.swift @@ -20,19 +20,20 @@ extension _Node.UnsafeHandle { @inlinable internal func _insertItem(_ item: __owned Element, at offset: Int) { assertMutable() - let count = itemCount - assert(offset >= 0 && offset <= count) + let c = itemCount + assert(offset >= 0 && offset <= c) let stride = MemoryLayout.stride assert(bytesFree >= stride) bytesFree &-= stride let start = _memory - .advanced(by: byteCapacity &- (count &+ 1) &* stride) - .bindMemory(to: Element.self, capacity: count &+ 1) + .advanced(by: byteCapacity &- (c &+ 1) &* stride) + .bindMemory(to: Element.self, capacity: 1) - start.moveInitialize(from: start + 1, count: offset) - (start + offset).initialize(to: item) + let prefix = c &- offset + start.moveInitialize(from: start + 1, count: prefix) + (start + prefix).initialize(to: item) } /// Insert `child` at `offset`. There must be enough free space in the node @@ -66,18 +67,19 @@ extension _Node.UnsafeHandle { @inlinable internal func _removeItem(at offset: Int) -> Element { assertMutable() - let count = itemCount - assert(offset >= 0 && offset < count) + let c = itemCount + assert(offset >= 0 && offset < c) let stride = MemoryLayout.stride bytesFree &+= stride let start = _memory - .advanced(by: byteCapacity &- stride &* count) + .advanced(by: byteCapacity &- stride &* c) .assumingMemoryBound(to: Element.self) - let q = start + offset + let prefix = c &- 1 &- offset + let q = start + prefix let item = q.move() - (start + 1).moveInitialize(from: start, count: offset) + (start + 1).moveInitialize(from: start, count: prefix) return item } @@ -249,19 +251,19 @@ extension _Node { insertItem((key, value), at: offset, bucket) return nil case .newCollision(let bucket, let offset): - let hash2 = read { _Hash($0[item: offset].key) } - if hash == hash2, hasSingletonItem { + let existingHash = read { _Hash($0[item: offset].key) } + if hash == existingHash, hasSingletonItem { // Convert current node to a collision node. ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) update { $0.collisionCount = 1 } - insertItem((key, value), at: 0, .invalid) + insertItem((key, value), at: 1, .invalid) } else { ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) - let item2 = removeItem(at: offset, bucket) + let existing = removeItem(at: offset, bucket) let node = _Node( level: level.descend(), - item1: (key, value), hash, - item2: item2, hash2) + item1: existing, existingHash, + item2: (key, value), hash) insertChild(node, bucket) } return nil diff --git a/Sources/PersistentCollections/Node/_Node+Storage.swift b/Sources/PersistentCollections/Node/_Node+Storage.swift index d0cfe646c..0b510f1dc 100644 --- a/Sources/PersistentCollections/Node/_Node+Storage.swift +++ b/Sources/PersistentCollections/Node/_Node+Storage.swift @@ -42,7 +42,7 @@ extension _Node { deinit { UnsafeHandle.update(self) { handle in handle._children.deinitialize() - handle._items.deinitialize() + handle.reverseItems.deinitialize() } } } @@ -162,7 +162,8 @@ extension _Node { let i = target._children.initialize(fromContentsOf: source._children) assert(i == target.childCount) - let j = target._items.initialize(fromContentsOf: source._items) + let j = target.reverseItems + .initialize(fromContentsOf: source.reverseItems) assert(j == target.itemCount) } } @@ -185,7 +186,8 @@ extension _Node { let i = target._children.moveInitialize(fromContentsOf: source._children) assert(i == target.childCount) - let j = target._items.moveInitialize(fromContentsOf: source._items) + let j = target.reverseItems + .moveInitialize(fromContentsOf: source.reverseItems) assert(j == target.itemCount) source.itemMap = .empty source.childMap = .empty diff --git a/Sources/PersistentCollections/Node/_Node.UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift similarity index 97% rename from Sources/PersistentCollections/Node/_Node.UnsafeHandle.swift rename to Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index 9afb62a76..c0b42237b 100644 --- a/Sources/PersistentCollections/Node/_Node.UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -171,7 +171,7 @@ extension _Node.UnsafeHandle { } @inlinable - internal var _items: UnsafeMutableBufferPointer { + internal var reverseItems: UnsafeMutableBufferPointer { let c = itemCount return UnsafeMutableBufferPointer(start: _itemsEnd - c, count: c) } @@ -179,7 +179,7 @@ extension _Node.UnsafeHandle { @inlinable internal func itemPtr(at offset: Int) -> UnsafeMutablePointer { assert(offset >= 0 && offset < itemCount) - return _itemsEnd.advanced(by: -(itemCount &- offset)) + return _itemsEnd.advanced(by: -1 &- offset) } @inlinable diff --git a/Sources/PersistentCollections/Node/_Node.swift b/Sources/PersistentCollections/Node/_Node.swift index d08893ca5..a9ad43129 100644 --- a/Sources/PersistentCollections/Node/_Node.swift +++ b/Sources/PersistentCollections/Node/_Node.swift @@ -38,10 +38,12 @@ internal struct _RawNode { /// The storage is arranged as shown below. /// /// ┌───┬───┬───┬───┬───┬──────────────────┬───┬───┬───┬───┐ -/// │ 0 │ 1 │ 2 │ 3 │ 4 │→ free space ←│ 0 │ 1 │ 2 │ 3 │ +/// │ 0 │ 1 │ 2 │ 3 │ 4 │→ free space ←│ 3 │ 2 │ 1 │ 0 │ /// └───┴───┴───┴───┴───┴──────────────────┴───┴───┴───┴───┘ /// children items /// +/// Note that the region for items grows downwards from the end. +/// /// Two 32-bit bitmaps are used to associate each child and item with their /// position in the hash table. The bucket occupied by the *n*th child in the /// buffer above is identified by position of the *n*th true bit in the child diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Sequence.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Sequence.swift index 4bc7b6d5e..750ad274b 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Sequence.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Sequence.swift @@ -18,7 +18,7 @@ extension PersistentDictionary: Sequence { // before any items within children. @usableFromInline - typealias _ItemBuffer = UnsafeBufferPointer + typealias _ItemBuffer = ReversedCollection> @usableFromInline typealias _ChildBuffer = UnsafeBufferPointer<_Node> @@ -44,7 +44,7 @@ extension PersistentDictionary: Sequence { // FIXME: This is illegal, as it escapes pointers to _root contents // outside the closure passed to `read`. :-( self._itemIterator = _root.read { - $0.hasItems ? $0._items.makeIterator() : nil + $0.hasItems ? $0.reverseItems.reversed().makeIterator() : nil } self._pathTop = _root.read { $0.hasChildren ? $0._children.makeIterator() : nil @@ -84,7 +84,7 @@ extension PersistentDictionary.Iterator: IteratorProtocol { _pathTop = nextNode.read { $0._children.makeIterator() } // 💥ILLEGAL } if nextNode.read({ $0.hasItems }) { - _itemIterator = nextNode.read { $0._items.makeIterator() } // 💥ILLEGAL + _itemIterator = nextNode.read { $0.reverseItems.reversed().makeIterator() } // 💥ILLEGAL return _itemIterator!.next() } } From 3e38184f2a2444ac4147b44e1b7a1f53d5f0b537 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:23:05 -0700 Subject: [PATCH 02/18] [PersistentCollections] Optimize _Node.== MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the key implements hashing properly, then this would be a clever way to try another hash seed to differentiate between colliding keys. However, if Key has proper hashing, then collision nodes are expected to be tiny, so it’s better to simply do naive comparisons. If the Key type has broken (i.e., partial) hashing, then loading colliding items into a dictionary would just make == even slower. So we’re better off comparing collidingh items via nested loops — so let’s just unconditionally do that. --- .../Node/_Node+Equatable.swift | 34 +++++++++---------- .../PersistentCollections Utils.swift | 31 ----------------- 2 files changed, 17 insertions(+), 48 deletions(-) delete mode 100644 Tests/PersistentCollectionsTests/PersistentCollections Utils.swift diff --git a/Sources/PersistentCollections/Node/_Node+Equatable.swift b/Sources/PersistentCollections/Node/_Node+Equatable.swift index bd90fa0e9..083e40c60 100644 --- a/Sources/PersistentCollections/Node/_Node+Equatable.swift +++ b/Sources/PersistentCollections/Node/_Node+Equatable.swift @@ -12,27 +12,27 @@ // TODO: `Equatable` needs more test coverage, apart from hash-collision smoke test extension _Node: Equatable where Value: Equatable { @inlinable - static func == (lhs: _Node, rhs: _Node) -> Bool { - if lhs.raw.storage === rhs.raw.storage { return true } - if lhs.isCollisionNode && rhs.isCollisionNode { - return lhs.read { lhs in - rhs.read { rhs in - let l = Dictionary( - uniqueKeysWithValues: lhs.reverseItems.lazy.map { ($0.key, $0.value) }) - let r = Dictionary( - uniqueKeysWithValues: rhs.reverseItems.lazy.map { ($0.key, $0.value) }) - return l == r + static func == (left: _Node, right: _Node) -> Bool { + if left.raw.storage === right.raw.storage { return true } + + guard left.count == right.count else { return false } + + if left.isCollisionNode { + guard right.isCollisionNode else { return false } + return left.read { lhs in + right.read { rhs in + let l = lhs.reverseItems + let r = rhs.reverseItems + guard l.count == r.count else { return false } + for i in l.indices { + guard r.contains(where: { $0 == l[i] }) else { return false } + } + return true } } } - return deepContentEquality(lhs, rhs) - } + guard !right.isCollisionNode else { return false } - @inlinable - internal static func deepContentEquality( - _ left: _Node, - _ right: _Node - ) -> Bool { guard left.count == right.count else { return false } return left.read { l in right.read { r in diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Utils.swift b/Tests/PersistentCollectionsTests/PersistentCollections Utils.swift deleted file mode 100644 index b4e6ad0bf..000000000 --- a/Tests/PersistentCollectionsTests/PersistentCollections Utils.swift +++ /dev/null @@ -1,31 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Collections open source project -// -// Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// -//===----------------------------------------------------------------------===// - -import _CollectionsTestSupport -import PersistentCollections - -extension LifetimeTracker { - func persistentDictionary( - keys: Keys - ) -> ( - dictionary: PersistentDictionary, LifetimeTracked>, - keys: [LifetimeTracked], - values: [LifetimeTracked] - ) - where Keys.Element == Int - { - let k = Array(keys) - let keys = self.instances(for: k) - let values = self.instances(for: k.map { $0 + 100 }) - let dictionary = PersistentDictionary(uniqueKeys: keys, values: values) - return (dictionary, keys, values) - } -} From edaadc9a8acff3084cb9c370c2a1267ef46bdc59 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:24:22 -0700 Subject: [PATCH 03/18] [PersistentCollections] _Level: Store the shift amount in an UInt8 --- .../PersistentCollections/Node/_Level.swift | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/Sources/PersistentCollections/Node/_Level.swift b/Sources/PersistentCollections/Node/_Level.swift index e347c4c46..04c33bbd7 100644 --- a/Sources/PersistentCollections/Node/_Level.swift +++ b/Sources/PersistentCollections/Node/_Level.swift @@ -13,11 +13,17 @@ @frozen internal struct _Level { @usableFromInline - internal var shift: UInt + internal var _shift: UInt8 + + @inlinable @inline(__always) + init(_shift: UInt8) { + self._shift = _shift + } @inlinable @inline(__always) init(shift: UInt) { - self.shift = shift + assert(shift <= UInt8.max) + self._shift = UInt8(truncatingIfNeeded: shift) } } @@ -28,8 +34,8 @@ extension _Level { } @inlinable @inline(__always) - internal static var _step: UInt { - UInt(bitPattern: _Bitmap.bitWidth) + internal static var _step: UInt8 { + UInt8(truncatingIfNeeded: _Bitmap.bitWidth) } @inlinable @inline(__always) @@ -38,27 +44,37 @@ extension _Level { } @inlinable @inline(__always) - internal var isAtRoot: Bool { shift == 0 } + internal var shift: UInt { UInt(truncatingIfNeeded: _shift) } + + @inlinable @inline(__always) + internal var isAtRoot: Bool { _shift == 0 } @inlinable @inline(__always) - internal var isAtBottom: Bool { shift >= UInt.bitWidth } + internal var isAtBottom: Bool { _shift >= UInt.bitWidth } @inlinable @inline(__always) internal func descend() -> _Level { // FIXME: Consider returning nil when we run out of bits - _Level(shift: shift &+ Self._step) + _Level(_shift: _shift &+ Self._step) } @inlinable @inline(__always) internal func ascend() -> _Level { assert(!isAtRoot) - return _Level(shift: shift &+ Self._step) + return _Level(_shift: _shift &- Self._step) } } extension _Level: Equatable { @inlinable @inline(__always) internal static func ==(left: Self, right: Self) -> Bool { - left.shift == right.shift + left._shift == right._shift + } +} + +extension _Level: Comparable { + @inlinable @inline(__always) + internal static func <(left: Self, right: Self) -> Bool { + left._shift < right._shift } } From 6dd02114b8b03f9493e3937b332ffa032df51c06 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:29:14 -0700 Subject: [PATCH 04/18] [PersistentCollections] Flesh out _RawNode, add _UnmanagedNode These provide a barebones, read-only, non-generic abstraction, with just enough functionality to allow navigating around in the tree structure. We will use these to implement path-based indices. --- .../Node/_Node+UnsafeHandle.swift | 12 +++ .../PersistentCollections/Node/_Node.swift | 28 +++--- .../Node/_RawNode+UnsafeHandle.swift | 75 ++++++++++++++++ .../PersistentCollections/Node/_RawNode.swift | 67 +++++++++++++++ .../Node/_UnmanagedNode.swift | 85 +++++++++++++++++++ 5 files changed, 251 insertions(+), 16 deletions(-) create mode 100644 Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift create mode 100644 Sources/PersistentCollections/Node/_RawNode.swift create mode 100644 Sources/PersistentCollections/Node/_UnmanagedNode.swift diff --git a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index c0b42237b..e11dfcda4 100644 --- a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -53,6 +53,18 @@ extension _Node.UnsafeHandle { } extension _Node.UnsafeHandle { + @inlinable @inline(__always) + static func read( + _ node: _UnmanagedNode, + _ body: (Self) throws -> R + ) rethrows -> R { + try node.ref._withUnsafeGuaranteedRef { storage in + try storage.withUnsafeMutablePointers { header, elements in + try body(Self(header, UnsafeMutableRawPointer(elements), isMutable: false)) + } + } + } + @inlinable @inline(__always) static func read( _ storage: _RawStorage, diff --git a/Sources/PersistentCollections/Node/_Node.swift b/Sources/PersistentCollections/Node/_Node.swift index a9ad43129..c57f682ee 100644 --- a/Sources/PersistentCollections/Node/_Node.swift +++ b/Sources/PersistentCollections/Node/_Node.swift @@ -9,22 +9,6 @@ // //===----------------------------------------------------------------------===// -@usableFromInline -@frozen -internal struct _RawNode { - @usableFromInline - internal var storage: _RawStorage - - @usableFromInline - internal var count: Int - - @inlinable - internal init(storage: _RawStorage, count: Int) { - self.storage = storage - self.count = count - } -} - /// A node in the hash tree, logically representing a hash table with /// 32 buckets, corresponding to a 5-bit slice of a full hash value. /// @@ -83,6 +67,18 @@ extension _Node { } } +extension _Node { + @inlinable @inline(__always) + internal var unmanaged: _UnmanagedNode { + _UnmanagedNode(raw.storage) + } + + @inlinable @inline(__always) + internal func isIdentical(to other: _UnmanagedNode) -> Bool { + raw.isIdentical(to: other) + } +} + extension _Node { @inlinable @inline(__always) internal func read(_ body: (UnsafeHandle) -> R) -> R { diff --git a/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift new file mode 100644 index 000000000..6017e0da2 --- /dev/null +++ b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +extension _RawNode { + @usableFromInline + @frozen + internal struct UnsafeHandle { + @usableFromInline + internal let _header: UnsafePointer<_StorageHeader> + + @usableFromInline + internal let _memory: UnsafeRawPointer + + @inlinable + internal init( + _ header: UnsafePointer<_StorageHeader>, + _ memory: UnsafeRawPointer + ) { + self._header = header + self._memory = memory + } + } +} + +extension _RawNode.UnsafeHandle { + @inline(__always) + internal var isCollisionNode: Bool { + _header.pointee.isCollisionNode + } + + @inline(__always) + internal var hasChildren: Bool { + _header.pointee.hasChildren + } + + @inline(__always) + internal var childCount: Int { + _header.pointee.childCount + } + + @inline(__always) + internal var hasItems: Bool { + _header.pointee.hasItems + } + + @inline(__always) + internal var itemCount: Int { + _header.pointee.itemCount + } + + + @inline(__always) + internal var _childrenStart: UnsafePointer<_RawNode> { + _memory.assumingMemoryBound(to: _RawNode.self) + } + + internal subscript(child offset: Int) -> _RawNode { + unsafeAddress { + assert(offset >= 0 && offset < childCount) + return _childrenStart + offset + } + } + + internal var _children: UnsafeBufferPointer<_RawNode> { + UnsafeBufferPointer(start: _childrenStart, count: childCount) + } +} diff --git a/Sources/PersistentCollections/Node/_RawNode.swift b/Sources/PersistentCollections/Node/_RawNode.swift new file mode 100644 index 000000000..e8ab9c910 --- /dev/null +++ b/Sources/PersistentCollections/Node/_RawNode.swift @@ -0,0 +1,67 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +@usableFromInline +@frozen +internal struct _RawNode { + @usableFromInline + internal var storage: _RawStorage + + @usableFromInline + internal var count: Int + + @inlinable + internal init(storage: _RawStorage, count: Int) { + self.storage = storage + self.count = count + } +} + +extension _RawNode { + @inline(__always) + internal func read(_ body: (UnsafeHandle) -> R) -> R { + storage.withUnsafeMutablePointers { header, elements in + body(UnsafeHandle(header, UnsafeRawPointer(elements))) + } + } +} + +extension _RawNode { + @inline(__always) + internal var unmanaged: _UnmanagedNode { + _UnmanagedNode(storage) + } + + @inlinable @inline(__always) + internal func isIdentical(to other: _UnmanagedNode) -> Bool { + other.ref.toOpaque() == Unmanaged.passUnretained(storage).toOpaque() + } +} + +extension _RawNode { + @usableFromInline + internal func validatePath(_ path: _UnsafePath) { + var l = _Level.top + var n = self.unmanaged + while l < path.level { + let offset = path.ancestors[l] + precondition(offset < n.childCount) + n = n.unmanagedChild(at: offset) + l = l.descend() + } + precondition(n == path.node) + if path._isItem { + precondition(path._nodeOffset < n.itemCount) + } else { + precondition(path._nodeOffset <= n.childCount) + } + } +} diff --git a/Sources/PersistentCollections/Node/_UnmanagedNode.swift b/Sources/PersistentCollections/Node/_UnmanagedNode.swift new file mode 100644 index 000000000..03ae07daf --- /dev/null +++ b/Sources/PersistentCollections/Node/_UnmanagedNode.swift @@ -0,0 +1,85 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +import _CollectionsUtilities + +@usableFromInline +@frozen +internal struct _UnmanagedNode { + @usableFromInline + internal var ref: Unmanaged<_RawStorage> + + @inlinable + internal init(_ storage: _RawStorage) { + self.ref = .passUnretained(storage) + } +} + +extension _UnmanagedNode: Equatable { + @inlinable + internal static func ==(left: Self, right: Self) -> Bool { + left.ref.toOpaque() == right.ref.toOpaque() + } +} + +extension _UnmanagedNode: CustomStringConvertible { + @usableFromInline + internal var description: String { + _addressString(for: ref.toOpaque()) + } +} + +extension _UnmanagedNode { + @inlinable @inline(__always) + internal func withRaw(_ body: (_RawStorage) -> R) -> R { + ref._withUnsafeGuaranteedRef(body) + } + + @inline(__always) + internal func read(_ body: (_RawNode.UnsafeHandle) -> R) -> R { + ref._withUnsafeGuaranteedRef { storage in + storage.withUnsafeMutablePointers { header, elements in + body(_RawNode.UnsafeHandle(header, UnsafeRawPointer(elements))) + } + } + } + + @inlinable + internal var hasItems: Bool { + withRaw { $0.header.hasItems } + } + + @inlinable + internal var hasChildren: Bool { + withRaw { $0.header.hasChildren } + } + + @inlinable + internal var itemCount: Int { + withRaw { $0.header.itemCount } + } + + @inlinable + internal var childCount: Int { + withRaw { $0.header.childCount } + } + + @inlinable + internal func unmanagedChild(at offset: Int) -> Self { + withRaw { raw in + assert(offset >= 0 && offset < raw.header.childCount) + return raw.withUnsafeMutablePointerToElements { p in + Self(p[offset].storage) + } + } + } +} + From 547ac9094f3cbed1c926643244abeced31e41058 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:30:33 -0700 Subject: [PATCH 05/18] [PersistentCollections] Rework basic node properties --- .../PersistentCollections/Node/_StorageHeader.swift | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Sources/PersistentCollections/Node/_StorageHeader.swift b/Sources/PersistentCollections/Node/_StorageHeader.swift index ac70a14f6..d0f568a55 100644 --- a/Sources/PersistentCollections/Node/_StorageHeader.swift +++ b/Sources/PersistentCollections/Node/_StorageHeader.swift @@ -52,6 +52,11 @@ extension _StorageHeader { } extension _StorageHeader { + @inlinable @inline(__always) + internal var isEmpty: Bool { + return itemMap.isEmpty && childMap.isEmpty + } + @inlinable @inline(__always) internal var isCollisionNode: Bool { !itemMap.intersection(childMap).isEmpty @@ -59,7 +64,7 @@ extension _StorageHeader { @inlinable @inline(__always) internal var hasChildren: Bool { - !isCollisionNode && !childMap.isEmpty + itemMap != childMap && !childMap.isEmpty } @inlinable @inline(__always) @@ -69,12 +74,14 @@ extension _StorageHeader { @inlinable internal var childCount: Int { - isCollisionNode ? 0 : childMap.count + itemMap == childMap ? 0 : childMap.count } @inlinable internal var itemCount: Int { - isCollisionNode ? collisionCount : itemMap.count + (itemMap == childMap + ? Int(truncatingIfNeeded: itemMap._value) + : itemMap.count) } @inlinable From 02133f12624a7925f20d503c51cecfb8cb0a71c6 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:37:36 -0700 Subject: [PATCH 06/18] [PersistentCollections] Implement path-based indices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Index type now represents a path into the tree, consisting of a series of child offsets from the root node, as well as the item offset within the final node. (This all fits in a couple of words, as the depth of the tree is fixed, and the offset path can always be compressed into a single `UInt` value.) To speed up basic operations, we also cache the final node as an unsafe-unowned reference. Dereferencing this is extremely unsafe, so the Index type carries an unsafe reference to the root node along with a mutation counter, to allow reliable(*) index validation. If index’s stored root is identical to the root of the collection, and they have the same version value, then we can assume that the index is still valid. --- .../Node/_AncestorOffsets.swift | 79 +++ .../Node/_UnsafePath.swift | 565 ++++++++++++++++++ .../PersistentDictionary+Collection.swift | 131 +++- .../PersistentDictionary.swift | 24 +- 4 files changed, 776 insertions(+), 23 deletions(-) create mode 100644 Sources/PersistentCollections/Node/_AncestorOffsets.swift create mode 100644 Sources/PersistentCollections/Node/_UnsafePath.swift diff --git a/Sources/PersistentCollections/Node/_AncestorOffsets.swift b/Sources/PersistentCollections/Node/_AncestorOffsets.swift new file mode 100644 index 000000000..b9b5257a5 --- /dev/null +++ b/Sources/PersistentCollections/Node/_AncestorOffsets.swift @@ -0,0 +1,79 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +@usableFromInline +@frozen +struct _AncestorOffsets { + @usableFromInline + internal var path: UInt + + @inlinable @inline(__always) + internal init(_ path: UInt) { + self.path = path + } +} + +extension _AncestorOffsets: Equatable { + @inlinable @inline(__always) + internal static func ==(left: Self, right: Self) -> Bool { + left.path == right.path + } +} + +extension _AncestorOffsets { + @inlinable @inline(__always) + internal static var empty: Self { Self(0) } +} + +extension _AncestorOffsets { + @inlinable @inline(__always) + internal subscript(_ level: _Level) -> Int { + get { + assert(level.shift < UInt.bitWidth) + return Int(bitPattern: (path &>> level.shift) & _Bucket.bitMask) + } + set { + assert(newValue < UInt.bitWidth) + assert(self[level] == 0) + path |= (UInt(bitPattern: newValue) &<< level.shift) + } + } + + @inlinable + internal func truncating(to level: _Level) -> _AncestorOffsets { + assert(level.shift <= UInt.bitWidth) + guard level.shift < UInt.bitWidth else { return self } + return _AncestorOffsets(path & ((1 &<< level.shift) &- 1)) + } + + @inlinable + internal mutating func clear(_ level: _Level) { + guard level.shift < UInt.bitWidth else { return } + path &= ~(_Bucket.bitMask &<< level.shift) + } + + @inlinable + internal func hasDataBelow(_ level: _Level) -> Bool { + guard level.shift < UInt.bitWidth else { return false } + return (path &>> level.shift) != 0 + } + + @inlinable + internal func isEqual(to other: Self, upTo level: _Level) -> Bool { + if level.isAtRoot { return true } + if level.isAtBottom { return self == other } + let s = UInt(UInt.bitWidth) - level.shift + let v1 = self.path &<< s + let v2 = other.path &<< s + return v1 == v2 + } +} + diff --git a/Sources/PersistentCollections/Node/_UnsafePath.swift b/Sources/PersistentCollections/Node/_UnsafePath.swift new file mode 100644 index 000000000..cbd2e9c21 --- /dev/null +++ b/Sources/PersistentCollections/Node/_UnsafePath.swift @@ -0,0 +1,565 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +@usableFromInline +internal struct _UnsafePath { + @usableFromInline + internal var ancestors: _AncestorOffsets + + @usableFromInline + internal var node: _UnmanagedNode + + @usableFromInline + internal var _nodeOffset: UInt32 + + @usableFromInline + internal var level: _Level + + @usableFromInline + internal var _isItem: Bool + + @usableFromInline + @_effects(releasenone) + internal init(root: __shared _RawNode) { + self.level = .top + self.ancestors = .empty + self.node = root.unmanaged + self._nodeOffset = 0 + self._isItem = root.storage.header.hasItems + } +} + +extension _UnsafePath { + internal init( + _ level: _Level, + _ ancestors: _AncestorOffsets, + _ node: _UnmanagedNode, + _ childOffset: Int + ) { + assert(childOffset >= 0 && childOffset < node.childCount) + self.level = level + self.ancestors = ancestors + self.node = node + self._nodeOffset = UInt32(truncatingIfNeeded: childOffset) + self._isItem = false + } +} + +extension _UnsafePath: Equatable { + @usableFromInline + @_effects(releasenone) + internal static func ==(left: Self, right: Self) -> Bool { + // Note: we don't compare nodes (node equality should follow from the rest) + left.level == right.level + && left.ancestors == right.ancestors + && left._nodeOffset == right._nodeOffset + && left._isItem == right._isItem + } +} + +extension _UnsafePath: Hashable { + @usableFromInline + @_effects(releasenone) + internal func hash(into hasher: inout Hasher) { + // Note: we don't hash nodes, as they aren't compared by ==, either. + hasher.combine(ancestors.path) + hasher.combine(_nodeOffset) + hasher.combine(level._shift) + hasher.combine(_isItem) + } +} + +extension _UnsafePath: Comparable { + @usableFromInline + @_effects(releasenone) + internal static func <(left: Self, right: Self) -> Bool { + // This implements a total ordering across paths based on the offset + // sequences they contain, corresponding to a preorder walk of the tree. + // + // Paths addressing items within a node are ordered before paths addressing + // a child node within the same node. + + var level: _Level = .top + while level < left.level, level < right.level { + let l = left.ancestors[level] + let r = right.ancestors[level] + guard l == r else { return l < r } + level = level.descend() + } + assert(level < left.level || !left.ancestors.hasDataBelow(level)) + assert(level < right.level || !right.ancestors.hasDataBelow(level)) + if level < right.level { + guard !left._isItem else { return true } + let l = left._nodeOffset + let r = right.ancestors[level] + return l < r + } + if level < left.level { + guard !right._isItem else { return false } + let l = left.ancestors[level] + let r = right._nodeOffset + return l < r + } + guard left._isItem == right._isItem else { return left._isItem } + return left._nodeOffset < right._nodeOffset + } +} + +extension _UnsafePath: CustomStringConvertible { + @usableFromInline + internal var description: String { + var d = "@" + var l: _Level = .top + while l < self.level { + d += ".\(self.ancestors[l])" + l = l.descend() + } + if isPlaceholder { + d += "[\(self._nodeOffset)]?" + } else if isOnItem { + d += "[\(self._nodeOffset)]" + } else if isOnChild { + d += ".\(self._nodeOffset)" + } else if isOnNodeEnd { + d += ".$(\(self._nodeOffset))" + } + return d + } +} + +extension _UnsafePath { + /// Returns true if this path addresses an item in the tree; otherwise returns + /// false. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @inlinable @inline(__always) + internal var isOnItem: Bool { + // Note: this may be true even if nodeOffset == itemCount (insertion paths). + _isItem + } + + /// Returns true if this path addresses the position following a node's last + /// valid item. Such paths can represent the place of an item that might be + /// inserted later; they do not occur while simply iterating over existing + /// items. + internal var isPlaceholder: Bool { + _isItem && _nodeOffset == node.itemCount + } + + /// Returns true if this path addresses a node in the tree; otherwise returns + /// false. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + internal var isOnChild: Bool { + !_isItem && _nodeOffset < node.childCount + } + + /// Returns true if this path addresses an empty slot within a node in a tree; + /// otherwise returns false. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + internal var isOnNodeEnd: Bool { + !_isItem && _nodeOffset == node.childCount + } + + @inlinable + internal var isOnLeftmostItem: Bool { + // We are on the leftmost item in the tree if we are currently + // addressing an item and the ancestors path is all zero bits. + _isItem && ancestors == .empty && _nodeOffset == 0 + } +} + +extension _UnsafePath { + @inlinable @inline(__always) + internal var nodeOffset: Int { + get { + Int(truncatingIfNeeded: _nodeOffset) + } + set { + assert(newValue >= 0 && newValue <= UInt32.max) + _nodeOffset = UInt32(truncatingIfNeeded: newValue) + } + } + + /// Returns an unmanaged reference to the child node this path is currently + /// addressing. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + internal var currentChild: _UnmanagedNode { + assert(isOnChild) + return node.unmanagedChild(at: nodeOffset) + } + + internal func childOffset(at level: _Level) -> Int { + assert(level < self.level) + return ancestors[level] + } + /// Returns the offset to the currently addressed item. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @inlinable @inline(__always) + internal var currentItemOffset: Int { + assert(isOnItem) + return nodeOffset + } +} + +extension _UnsafePath { + /// Positions this path on the item with the specified offset within its + /// current node. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @inlinable + internal mutating func selectItem(at offset: Int) { + // As a special exception, this allows offset to equal the item count. + // This can happen for paths that address the position a new item might be + // inserted later. + assert(offset >= 0 && offset <= node.itemCount) + nodeOffset = offset + _isItem = true + } + + /// Positions this path on the child with the specified offset within its + /// current node, without descending into it. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @inlinable + internal mutating func selectChild(at offset: Int) { + // As a special exception, this allows offset to equal the child count. + // This is equivalent to a call to `selectEnd()`. + assert(offset >= 0 && offset <= node.childCount) + nodeOffset = offset + _isItem = false + } + + /// Positions this path on the empty slot at the end of its current node. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @usableFromInline + @_effects(releasenone) + internal mutating func selectEnd() { + nodeOffset = node.childCount + _isItem = false + } + + /// Descend onto the first path within the currently selected child. + /// (Either the first item if it exists, or the first child. If the child + /// is an empty node (which should not happen in a hash tree), then this + /// selects the empty slot at the end of it. + /// + /// - Note: This method needs to resolve the unmanaged node reference + /// that is stored in the path. It is up to the caller to ensure this will + /// never get called when the node is no longer valid; otherwise this will + /// trigger undefined behavior. + @usableFromInline + @_effects(releasenone) + internal mutating func descend() { + self.node = currentChild + self.ancestors[level] = nodeOffset + self.nodeOffset = 0 + self._isItem = node.hasItems + self.level = level.descend() + } + + internal mutating func ascendToNearestAncestor( + under root: _RawNode, + where test: (_UnmanagedNode, Int) -> Bool + ) -> Bool { + if self.level.isAtRoot { return false } + var best: _UnsafePath? = nil + var n = root.unmanaged + var l: _Level = .top + while l < self.level { + let offset = self.ancestors[l] + if test(n, offset) { + best = _UnsafePath(l, self.ancestors.truncating(to: l), n, offset) + } + n = n.unmanagedChild(at: offset) + l = l.descend() + } + guard let best = best else { return false } + self = best + return true + } + + internal mutating func ascend(under root: _RawNode) { + assert(!self.level.isAtRoot) + var n = root.unmanaged + var l: _Level = .top + while l.descend() < self.level { + n = n.unmanagedChild(at: self.ancestors[l]) + l = l.descend() + } + assert(l.descend() == self.level) + self._isItem = false + self.nodeOffset = self.ancestors[l] + let oldNode = self.node + self.node = n + self.ancestors.clear(l) + self.level = l + assert(self.currentChild == oldNode) + } +} + +extension _UnsafePath { + mutating func selectNextItem() -> Bool { + assert(isOnItem) + _nodeOffset &+= 1 + if _nodeOffset < node.itemCount { return true } + _nodeOffset = 0 + _isItem = false + return false + } + + mutating func selectNextChild() -> Bool { + assert(!isOnItem) + let childCount = node.childCount + guard _nodeOffset < childCount else { return false } + _nodeOffset &+= 1 + return _nodeOffset < childCount + } +} + +extension _UnsafePath { + @usableFromInline + @_effects(releasenone) + internal mutating func descendToLeftMostItem() { + while isOnChild { + descend() + } + } + + internal mutating func descendToRightMostItem() { + assert(isOnChild) + while true { + descend() + let childCount = node.childCount + guard childCount > 0 else { break } + selectChild(at: childCount - 1) + } + let itemCount = node.itemCount + assert(itemCount > 0) + selectItem(at: itemCount - 1) + } + + @usableFromInline + @_effects(releasenone) + internal mutating func findSuccessorItem(under root: _RawNode) -> Bool { + guard isOnItem else { return false } + if selectNextItem() { return true } + if node.hasChildren { + descendToLeftMostItem() + assert(isOnItem) + return true + } + if ascendToNearestAncestor( + under: root, where: { $1 &+ 1 < $0.childCount } + ) { + let r = selectNextChild() + assert(r) + descendToLeftMostItem() + assert(isOnItem) + return true + } + self = _UnsafePath(root: root) + self.selectEnd() + return true + } + + @usableFromInline + @_effects(releasenone) + internal mutating func findPredecessorItem(under root: _RawNode) -> Bool { + switch (isOnItem, nodeOffset > 0) { + case (true, true): + selectItem(at: nodeOffset &- 1) + return true + case (false, true): + selectChild(at: nodeOffset &- 1) + descendToRightMostItem() + return true + case (false, false): + if node.hasItems { + selectItem(at: node.itemCount &- 1) + return true + } + case (true, false): + break + } + guard + ascendToNearestAncestor(under: root, where: { $0.hasItems || $1 > 0 }) + else { return false } + + if nodeOffset > 0 { + selectChild(at: nodeOffset &- 1) + descendToRightMostItem() + return true + } + if node.hasItems { + selectItem(at: node.itemCount &- 1) + return true + } + return false + } +} + +extension _RawNode { + internal func preorderPosition(_ level: _Level, of path: _UnsafePath) -> Int { + if path.isOnNodeEnd { return count } + assert(path.isOnItem) + if level < path.level { + let childOffset = path.childOffset(at: level) + return read { + let prefix = $0._children[.. Int { + assert(level.isAtRoot) + if start.isOnNodeEnd { + // Shortcut: distance from end. + return preorderPosition(level, of: end) - count + } + if end.isOnNodeEnd { + // Shortcut: distance to end. + return count - preorderPosition(level, of: start) + } + assert(start.isOnItem) + assert(end.isOnItem) + if start.level == end.level, start.ancestors == end.ancestors { + // Shortcut: the paths are under the same node. + precondition(start.node == end.node, "Internal index validation error") + return end.currentItemOffset - start.currentItemOffset + } + if + start.level < end.level, + start.ancestors.isEqual(to: end.ancestors, upTo: start.level) + { + // Shortcut: start's node is an ancestor of end's position. + return start.node._distance( + start.level, fromItemAtOffset: start.currentItemOffset, to: end) + } + if start.ancestors.isEqual(to: end.ancestors, upTo: end.level) { + // Shortcut: end's node is an ancestor of start's position. + return -end.node._distance( + end.level, fromItemAtOffset: end.currentItemOffset, to: start) + } + // No shortcuts -- the two paths are in different subtrees. + // Start descending from the root to look for the closest common + // ancestor. + if start < end { + return _distance(level, from: start, to: end) + } + return -_distance(level, from: end, to: start) + } + + internal func _distance( + _ level: _Level, from start: _UnsafePath, to end: _UnsafePath + ) -> Int { + assert(start < end) + assert(level < start.level) + assert(level < end.level) + let offset1 = start.childOffset(at: level) + let offset2 = end.childOffset(at: level) + if offset1 == offset2 { + return read { + $0[child: offset1]._distance(level.descend(), from: start, to: end) + } + } + return read { + let children = $0._children + let d1 = children[offset1].preorderPosition(level.descend(), of: start) + let d2 = children[offset1 &+ 1 ..< offset2].reduce(0) { $0 + $1.count } + let d3 = children[offset2].preorderPosition(level.descend(), of: end) + return (children[offset1].count - d1) + d2 + d3 + } + } +} + +extension _UnmanagedNode { + internal func _distance( + _ level: _Level, fromItemAtOffset start: Int, to end: _UnsafePath + ) -> Int { + read { + assert(start >= 0 && start < $0.itemCount) + assert(level < end.level) + let childOffset = end.childOffset(at: level) + let children = $0._children + let prefix = children[.. (found: Bool, path: _UnsafePath) { + var path = _UnsafePath(root: raw) + while true { + let r = UnsafeHandle.read(path.node) { + $0.find(path.level, key, hash, forInsert: false) + } + switch r { + case .found(_, let offset): + path.selectItem(at: offset) + return (true, path) + case .notFound(_, let offset), .newCollision(_, let offset): + path.selectItem(at: offset) + return (false, path) + case .expansion: + return (false, path) + case .descend(_, let offset): + path.selectChild(at: offset) + path.descend() + } + } + } +} diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift index a13507e2f..e39fb3b44 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift @@ -9,43 +9,134 @@ // //===----------------------------------------------------------------------===// -extension PersistentDictionary: Collection { - public struct Index: Comparable { +extension PersistentDictionary { + @frozen + public struct Index { @usableFromInline - internal let _value: Int + internal let _root: _UnmanagedNode @usableFromInline - internal init(_value: Int) { - self._value = _value - } + internal var _version: UInt + + @usableFromInline + internal var _path: _UnsafePath - @inlinable - public static func < (lhs: Self, rhs: Self) -> Bool { - lhs._value < rhs._value + @inlinable @inline(__always) + internal init( + _root: _UnmanagedNode, version: UInt, path: _UnsafePath + ) { + self._root = _root + self._version = version + self._path = path } } +} +extension PersistentDictionary.Index: Equatable { @inlinable - public var isEmpty: Bool { _root.count == 0 } + public static func ==(left: Self, right: Self) -> Bool { + precondition( + left._root == right._root && left._version == right._version, + "Indices from different dictionary values aren't comparable") + return left._path == right._path + } +} +extension PersistentDictionary.Index: Hashable { @inlinable - public var count: Int { _root.count } + public func hash(into hasher: inout Hasher) { + hasher.combine(_path) + } +} + +extension PersistentDictionary.Index: Comparable { + public static func <(left: Self, right: Self) -> Bool { + precondition( + left._root == right._root && left._version == right._version, + "Indices from different dictionary values aren't comparable") + return left._path < right._path + } +} + +extension PersistentDictionary.Index: CustomStringConvertible { + public var description: String { + _path.description + } +} + +extension PersistentDictionary: BidirectionalCollection { + @inlinable + public var isEmpty: Bool { + _root.count == 0 + } @inlinable - public var startIndex: Index { Index(_value: 0) } + public var count: Int { + _root.count + } @inlinable - public var endIndex: Index { Index(_value: count) } - + public var startIndex: Index { + var path = _UnsafePath(root: _root.raw) + path.descendToLeftMostItem() + return Index(_root: _root.unmanaged, version: _version, path: path) + } + @inlinable - public func index(after i: Index) -> Index { - Index(_value: i._value + 1) + public var endIndex: Index { + var path = _UnsafePath(root: _root.raw) + path.selectEnd() + return Index(_root: _root.unmanaged, version: _version, path: path) } - + + @inlinable @inline(__always) + internal func _isValid(_ i: Index) -> Bool { + _root.isIdentical(to: i._root) && i._version == self._version + } + /// Accesses the key-value pair at the specified position. @inlinable - public subscript(position: Index) -> Element { - _root.item(position: position._value) + public subscript(i: Index) -> Element { + precondition(_isValid(i), "Invalid index") + precondition(i._path.isOnItem, "Can't get element at endIndex") + return _Node.UnsafeHandle.read(i._path.node) { + $0[item: i._path.currentItemOffset] + } } -} + @inlinable + public func formIndex(after i: inout Index) { + precondition(_isValid(i), "Invalid index") + guard i._path.findSuccessorItem(under: _root.raw) else { + preconditionFailure("The end index has no successor") + } + } + + @inlinable + public func formIndex(before i: inout Index) { + precondition(_isValid(i), "Invalid index") + guard i._path.findPredecessorItem(under: _root.raw) else { + preconditionFailure("The start index has no predecessor") + } + } + + @inlinable @inline(__always) + public func index(after i: Index) -> Index { + var i = i + formIndex(after: &i) + return i + } + + @inlinable @inline(__always) + public func index(before i: Index) -> Index { + var i = i + formIndex(before: &i) + return i + } + + @inlinable + public func distance(from start: Index, to end: Index) -> Int { + precondition(_isValid(start) && _isValid(end), "Invalid index") + return _root.raw.distance(.top, from: start._path, to: end._path) + } +} diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index e3e213242..e4381f698 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -16,16 +16,32 @@ public struct PersistentDictionary where Key: Hashable { @usableFromInline var _root: _Node + /// The version number of this instance, used for quick index validation. + /// This is initialized to a (very weakly) random value and it gets + /// incremented on every mutation that needs to invalidate indices. + @usableFromInline + var _version: UInt + @inlinable - internal init(_root: _Node) { + internal init(_root: _Node, version: UInt) { self._root = _root + self._version = version + } + + @inlinable + internal init(_new: _Node) { + self._root = _new + // Ideally we would simply just generate a true random number, but the + // memory address of the root node is a reasonable substitute. + let address = Unmanaged.passUnretained(_root.raw.storage).toOpaque() + self._version = UInt(bitPattern: address) } } extension PersistentDictionary { @inlinable public init() { - self.init(_root: _Node(storage: _emptySingleton, count: 0)) + self.init(_new: _Node(storage: _emptySingleton, count: 0)) } @inlinable @@ -101,7 +117,9 @@ extension PersistentDictionary { /// Returns the index for the given key. @inlinable public func index(forKey key: Key) -> Index? { - _root.position(forKey: key, .top, _Hash(key)).map { Index(_value: $0) } + let (found, path) = _root.path(to: key, hash: _Hash(key)) + guard found else { return nil } + return Index(_root: _root.unmanaged, version: _version, path: path) } @inlinable From 16393ea2c8c21a4b882ed6c00e039fc893c46ce7 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:38:07 -0700 Subject: [PATCH 07/18] [PersistentCollections] Allow dumping hash trees in iteration order --- .../Node/_Node+Debugging.swift | 50 +++++++++++++++---- .../PersistentDictionary+Debugging.swift | 4 +- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/Sources/PersistentCollections/Node/_Node+Debugging.swift b/Sources/PersistentCollections/Node/_Node+Debugging.swift index 794439ec8..053a8e3b2 100644 --- a/Sources/PersistentCollections/Node/_Node+Debugging.swift +++ b/Sources/PersistentCollections/Node/_Node+Debugging.swift @@ -14,22 +14,28 @@ import _CollectionsUtilities extension _Node { @usableFromInline internal func dump( - firstPrefix: String = "", restPrefix: String = "", limit: Int = Int.max + iterationOrder: Bool = false, + limit: Int = Int.max, + firstPrefix: String = "", + restPrefix: String = "", + depth: Int = 0 ) { read { $0.dump( + iterationOrder: iterationOrder, + limit: limit, + extra: "count: \(count), ", firstPrefix: firstPrefix, restPrefix: restPrefix, - extra: "count: \(count), ", - limit: limit) + depth: depth) } } } extension _Node.Storage { @usableFromInline - final internal func dump() { - UnsafeHandle.read(self) { $0.dump() } + final internal func dump(iterationOrder: Bool = false) { + UnsafeHandle.read(self) { $0.dump(iterationOrder: iterationOrder) } } } @@ -42,11 +48,22 @@ extension _Node.UnsafeHandle { @usableFromInline internal func dump( + iterationOrder: Bool = false, + limit: Int = .max, + extra: String = "", firstPrefix: String = "", restPrefix: String = "", - extra: String = "", - limit: Int = Int.max + depth: Int = 0 ) { + var firstPrefix = firstPrefix + var restPrefix = restPrefix + if iterationOrder && depth == 0 { + firstPrefix += "@" + restPrefix += "@" + } + if iterationOrder { + firstPrefix += " " + } print(""" \(firstPrefix)\(isCollisionNode ? "CollisionNode" : "Node")(\ at: \(_addressString(for: _header)), \ @@ -55,7 +72,20 @@ extension _Node.UnsafeHandle { freeBytes: \(bytesFree)) """) guard limit > 0 else { return } - if isCollisionNode { + if iterationOrder { + for offset in 0 ..< itemCount { + print(" \(restPrefix)[\(offset)] \(_itemString(at: offset))") + } + for offset in 0 ..< childCount { + self[child: offset].dump( + iterationOrder: true, + limit: limit - 1, + firstPrefix: " \(restPrefix).\(offset)", + restPrefix: " \(restPrefix).\(offset)", + depth: depth + 1) + } + } + else if isCollisionNode { for offset in 0 ..< itemCount { print("\(restPrefix)[\(offset)] \(_itemString(at: offset))") } @@ -70,9 +100,11 @@ extension _Node.UnsafeHandle { itemOffset += 1 } else if childMap.contains(bucket) { self[child: childOffset].dump( + iterationOrder: false, + limit: limit - 1, firstPrefix: "\(restPrefix) \(bucketStr) ", restPrefix: "\(restPrefix) ", - limit: limit - 1) + depth: depth + 1) childOffset += 1 } } diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift index 9b20007e8..086e70f86 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift @@ -12,7 +12,7 @@ import _CollectionsUtilities extension PersistentDictionary { - public func _dump() { - _root.dump() + public func _dump(iterationOrder: Bool = false) { + _root.dump(iterationOrder: iterationOrder) } } From bf3617e379b9fef8b1d7d5643e5e37180f8d8ba8 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Thu, 15 Sep 2022 20:41:18 -0700 Subject: [PATCH 08/18] [PersistentCollections] Work on testing a bit; add fixtures The new fixtures provide an easier way to construct sample hash trees with specific structure, to run tests on them. Fixtures are able to determine the expected ordering for each sample tree, which comes super useful for verifying collection conformances. --- .../Colliders.swift | 102 +++++ Tests/PersistentCollectionsTests/Hash.swift | 74 ++++ .../PersistentCollections Fixtures.swift | 259 +++++++++++++ .../PersistentCollections Smoke Tests.swift | 364 +++++++++--------- .../PersistentCollections Tests.swift | 260 ++++++++++++- .../Utilities.swift | 31 ++ .../_CollidableInt.swift | 69 ---- 7 files changed, 901 insertions(+), 258 deletions(-) create mode 100644 Tests/PersistentCollectionsTests/Colliders.swift create mode 100644 Tests/PersistentCollectionsTests/Hash.swift create mode 100644 Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift create mode 100644 Tests/PersistentCollectionsTests/Utilities.swift delete mode 100644 Tests/PersistentCollectionsTests/_CollidableInt.swift diff --git a/Tests/PersistentCollectionsTests/Colliders.swift b/Tests/PersistentCollectionsTests/Colliders.swift new file mode 100644 index 000000000..ea09792f3 --- /dev/null +++ b/Tests/PersistentCollectionsTests/Colliders.swift @@ -0,0 +1,102 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +/// A type with manually controlled hash values, for easy testing of collision +/// scenarios. +struct Collider: + Hashable, CustomStringConvertible, CustomDebugStringConvertible +{ + var identity: Int + var hash: Hash + + init(_ identity: Int, _ hash: Hash) { + self.identity = identity + self.hash = hash + } + + init(_ identity: Int) { + self.identity = identity + self.hash = Hash(identity) + } + + init(_ identity: String) { + self.hash = Hash(identity)! + self.identity = hash.value + } + + static func ==(left: Self, right: Self) -> Bool { + guard left.identity == right.identity else { return false } + precondition(left.hash == right.hash) + return true + } + + func hash(into hasher: inout Hasher) { + hasher.combine(hash.value) + } + + var description: String { + "\(identity)" + } + + var debugDescription: String { + "\(identity)-#\(hash)" + } +} + +/// A type with precisely controlled hash values. This can be used to set up +/// hash tables with fully deterministic contents. +struct RawCollider: + Hashable, CustomStringConvertible, CustomDebugStringConvertible +{ + var identity: Int + var hash: Hash + + init(_ identity: Int, _ hash: Hash) { + self.identity = identity + self.hash = hash + } + + init(_ identity: Int) { + self.identity = identity + self.hash = Hash(identity) + } + + init(_ identity: String) { + self.hash = Hash(identity)! + self.identity = hash.value + } + + static func ==(left: Self, right: Self) -> Bool { + guard left.identity == right.identity else { return false } + precondition(left.hash == right.hash) + return true + } + + var hashValue: Int { + fatalError("Don't") + } + + func hash(into hasher: inout Hasher) { + fatalError("Don't") + } + + func _rawHashValue(seed: Int) -> Int { + hash.value + } + + var description: String { + return "\(identity)" + } + + var debugDescription: String { + "\(identity)-#\(hash)" + } +} diff --git a/Tests/PersistentCollectionsTests/Hash.swift b/Tests/PersistentCollectionsTests/Hash.swift new file mode 100644 index 000000000..708f4e7e9 --- /dev/null +++ b/Tests/PersistentCollectionsTests/Hash.swift @@ -0,0 +1,74 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +/// An abstract representation of a hash value. +struct Hash { + var value: Int + + init(_ value: Int) { + self.value = value + } + + init(_ value: UInt) { + self.value = Int(bitPattern: value) + } +} + + +extension Hash { + static var bucketBitWidth: Int { 5 } + static var bucketCount: Int { 1 << bucketBitWidth } + static var bitWidth: Int { UInt.bitWidth } +} + +extension Hash: Equatable { + static func ==(left: Self, right: Self) -> Bool { + left.value == right.value + } +} + +extension Hash: CustomStringConvertible { + var description: String { + // Print hash values in radix 32 & reversed, so that the path in the hash + // tree is readily visible. + let p = String( + UInt(bitPattern: value), + radix: Self.bucketCount, + uppercase: true) + #if false // The zeroes look overwhelmingly long in this context + let c = (Self.bitWidth + Self.bucketBitWidth - 1) / Self.bucketBitWidth + let path = String(repeating: "0", count: Swift.max(0, c - p.count)) + p + return String(path.reversed()) + #else + return p + #endif + } +} + +extension Hash: LosslessStringConvertible { + init?(_ description: String) { + let s = String(description.reversed()) + guard let hash = UInt(s, radix: 32) else { return nil } + self.init(Int(bitPattern: hash)) + } +} + +extension Hash: ExpressibleByIntegerLiteral { + init(integerLiteral value: UInt) { + self.init(value) + } +} + +extension Hash: ExpressibleByStringLiteral { + init(stringLiteral value: String) { + self.init(value)! + } +} diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift b/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift new file mode 100644 index 000000000..a91116dfb --- /dev/null +++ b/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift @@ -0,0 +1,259 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +import _CollectionsTestSupport +import PersistentCollections + +/// A list of example trees to use while testing persistent hash maps. +/// +/// Each example has a name and a list of path specifications or collisions. +/// +/// A path spec is an ASCII `String` representing the hash of a key/value pair, +/// a.k.a a path in the prefix tree. Each character in the string identifies +/// a bucket index of a tree node, starting from the root. +/// (Encoded in radix 32, with digits 0-9 followed by letters. In order to +/// prepare for a potential reduction in the maximum node size, it is best to +/// keep the digits in the range 0-F.) The prefix tree's depth is limited by the +/// size of hash values. +/// +/// For example, the string "5A" corresponds to a key/value pair +/// that is in bucket 10 of a second-level node that is found at bucket 5 +/// of the root node. +/// +/// Hash collisions are modeled by strings of the form `*` where +/// `` is a path specification, and `` is the number of times that +/// path needs to be repeated. (To implement the collisions, the path is +/// extended with an infinite number of zeroes.) +/// +/// To generate input data from these fixtures, the items are sorted into +/// the same order as we expect a preorder walk would visit them in the +/// resulting tree. The resulting ordering is then used to insert key/value +/// pairs into the map, with sequentially increasing keys. +let _fixtures: KeyValuePairs = [ + "empty": [], + "single-item": [ + "A" + ], + "single-node": [ + "0", + "1", + "2", + "3", + "4", + "A", + "B", + "C", + "D", + ], + "few-collisions": [ + "42*5" + ], + "many-collisions": [ + "42*40" + ], + "few-different-collisions": [ + "1*3", + "21*3", + "22*3", + "3*3", + ], + "everything-on-the-2nd-level": [ + "00", "01", "02", "03", "04", + "10", "11", "12", "13", "14", + "20", "21", "22", "23", "24", + "30", "31", "32", "33", "34", + ], + "two-levels-mixed": [ + "00", "01", + "2", + "30", "33", + "4", + "5", + "60", "61", "66", + "71", "75", "77", + "8", + "94", "98", "9A", + "A3", "A4", + ], + "vee": [ + "11110", + "11115", + "11119", + "1111B", + "66664", + "66667", + ], + "fork": [ + "31110", + "31115", + "31119", + "3111B", + "36664", + "36667", + ], + "chain": [ + "0", + "10", + "110", + "1110", + "11110", + "11111", + ], + "nested": [ + "50", + "51", + "520", + "521", + "5220", + "5221", + "52220", + "52221", + "522220", + "522221", + "5222220", + "5222221", + "52222220", + "52222221", + "522222220", + "522222221", + "5222222220", + "5222222221", + "5222222222", + "5222222223", + "522222223", + "522222224", + "52222223", + "52222224", + "5222223", + "5222224", + "522223", + "522224", + "52223", + "52224", + "5223", + "5224", + "53", + "54", + ], + "deep": [ + "0", + + // Deeply nested children with only the leaf containing items + "1234560", + "1234561", + "1234562", + "1234563", + + "22", + "25", + ], +] + +struct Fixture { + typealias Element = (key: Key, value: Value) + + let title: String + let items: [Element] + + init(title: String, items: [Element]) { + self.title = title + self.items = items + } +} + +func withEachFixture( + body: (Fixture) -> Void +) { + for (name, fixture) in _fixtures { + let entry = TestContext.current.push("fixture: \(name)") + defer { TestContext.current.pop(entry) } + + let maxDepth = 15 // Larger than the actual maximum tree depth + var paths: [String] = [] + var seen: Set = [] + for item in fixture { + let path: String + if let i = item.unicodeScalars.firstIndex(of: "*") { + // We need to extend the path of collisions with zeroes to + // make sure they sort correctly. + let p = String(item.unicodeScalars.prefix(upTo: i)) + guard let count = Int(item.suffix(from: i).dropFirst(), radix: 10) + else { fatalError("Invalid item: '\(item)'") } + path = p.appending(String(repeating: "0", count: maxDepth - p.unicodeScalars.count)) + paths.append(contentsOf: repeatElement(path, count: count)) + } else { + path = item + paths.append(path) + } + + let normalized = path + .uppercased() + .appending(String(repeating: "0", count: maxDepth - path.unicodeScalars.count)) + if !seen.insert(normalized).inserted { + fatalError("Unexpected duplicate path: '\(path)'") + } + } + + // Sort paths into the order that we expect items will appear in the + // dictionary. + paths.sort { a, b in + var a = a.unicodeScalars[...] + var b = b.unicodeScalars[...] + // Ignore common prefix + while !a.isEmpty && !b.isEmpty && a.first == b.first { + a = a.dropFirst() + b = b.dropFirst() + } + switch (a.isEmpty, b.isEmpty) { + case (true, true): return false // a == b + case (true, false): return true // a < b, like 44 < 443 + case (false, true): return false // a > b like 443 > 44 + case (false, false): break + } + if a.count == 1 && b.count > 1 { + return true // a < b, like 45 < 423 + } + if b.count == 1 && a.count > 1 { + return false // a > b, like 423 > 45 + } + return a.first! < b.first! + } + + var items: [(key: RawCollider, value: Int)] = [] + items.reserveCapacity(paths.count) + var id = 0 + for path in paths { + let hash = Hash(path)! + let key = RawCollider(id, hash) + let value = 100 + id + items.append((key, value)) + id += 1 + } + + print(name) + body(Fixture(title: name, items: items)) + } +} + + +extension LifetimeTracker { + func persistentDictionary( + for fixture: [(key: RawCollider, value: Int)] + ) -> ( + map: PersistentDictionary, LifetimeTracked>, + ref: [(key: LifetimeTracked, value: LifetimeTracked)] + ) { + let ref = fixture.map { (key, value) in + (key: self.instance(for: key), value: self.instance(for: value)) + } + return (PersistentDictionary(uniqueKeysWithValues: ref), ref) + } +} diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Smoke Tests.swift b/Tests/PersistentCollectionsTests/PersistentCollections Smoke Tests.swift index 1a2a80f91..b4029b6b9 100644 --- a/Tests/PersistentCollectionsTests/PersistentCollections Smoke Tests.swift +++ b/Tests/PersistentCollectionsTests/PersistentCollections Smoke Tests.swift @@ -12,7 +12,7 @@ import _CollectionsTestSupport @testable import PersistentCollections -final class CapsuleSmokeTests: CollectionTestCase { +final class PersistentDictionarySmokeTests: CollectionTestCase { func testSubscriptAdd() { var map: PersistentDictionary = [1: "a", 2: "b"] @@ -77,27 +77,27 @@ final class CapsuleSmokeTests: CollectionTestCase { } func testTriggerOverwrite2() { - var res1: PersistentDictionary = [:] - res1.updateValue("a", forKey: CollidableInt(10, 01)) // in-place - res1.updateValue("a", forKey: CollidableInt(11, 33)) // in-place - res1.updateValue("b", forKey: CollidableInt(20, 02)) // in-place + var res1: PersistentDictionary = [:] + res1.updateValue("a", forKey: Collider(10, 01)) // in-place + res1.updateValue("a", forKey: Collider(11, 33)) // in-place + res1.updateValue("b", forKey: Collider(20, 02)) // in-place - res1.updateValue("x", forKey: CollidableInt(10, 01)) // in-place - res1.updateValue("x", forKey: CollidableInt(11, 33)) // in-place - res1.updateValue("y", forKey: CollidableInt(20, 02)) // in-place + res1.updateValue("x", forKey: Collider(10, 01)) // in-place + res1.updateValue("x", forKey: Collider(11, 33)) // in-place + res1.updateValue("y", forKey: Collider(20, 02)) // in-place - var res2: PersistentDictionary = res1 - res2.updateValue("a", forKey: CollidableInt(10, 01)) // triggers COW - res2.updateValue("a", forKey: CollidableInt(11, 33)) // in-place - res2.updateValue("b", forKey: CollidableInt(20, 02)) // in-place + var res2: PersistentDictionary = res1 + res2.updateValue("a", forKey: Collider(10, 01)) // triggers COW + res2.updateValue("a", forKey: Collider(11, 33)) // in-place + res2.updateValue("b", forKey: Collider(20, 02)) // in-place - expectEqual(res1[CollidableInt(10, 01)], "x") - expectEqual(res1[CollidableInt(11, 33)], "x") - expectEqual(res1[CollidableInt(20, 02)], "y") + expectEqual(res1[Collider(10, 01)], "x") + expectEqual(res1[Collider(11, 33)], "x") + expectEqual(res1[Collider(20, 02)], "y") - expectEqual(res2[CollidableInt(10, 01)], "a") - expectEqual(res2[CollidableInt(11, 33)], "a") - expectEqual(res2[CollidableInt(20, 02)], "b") + expectEqual(res2[Collider(10, 01)], "a") + expectEqual(res2[Collider(11, 33)], "a") + expectEqual(res2[Collider(20, 02)], "b") } @@ -105,15 +105,15 @@ final class CapsuleSmokeTests: CollectionTestCase { let upperBound = 1_000 // Populating `map1` - var map1: PersistentDictionary = [:] + var map1: PersistentDictionary = [:] for index in 0.. = map1 + var map2: PersistentDictionary = map1 for index in 0.. = map2 + var map3: PersistentDictionary = map2 for index in 0.. = [:] + let map: PersistentDictionary = [:] var res12 = map - res12[CollidableInt(1, 1)] = CollidableInt(1, 1) - res12[CollidableInt(2, 1)] = CollidableInt(2, 1) + res12[Collider(1, 1)] = Collider(1, 1) + res12[Collider(2, 1)] = Collider(2, 1) var res13 = map - res13[CollidableInt(1, 1)] = CollidableInt(1, 1) - res13[CollidableInt(3, 1)] = CollidableInt(3, 1) + res13[Collider(1, 1)] = Collider(1, 1) + res13[Collider(3, 1)] = Collider(3, 1) var res31 = map - res31[CollidableInt(3, 1)] = CollidableInt(3, 1) - res31[CollidableInt(1, 1)] = CollidableInt(1, 1) + res31[Collider(3, 1)] = Collider(3, 1) + res31[Collider(1, 1)] = Collider(1, 1) expectEqual(res13, res31) expectNotEqual(res13, res12) @@ -191,9 +191,9 @@ final class CapsuleSmokeTests: CollectionTestCase { func testCountForCopyOnWriteInsertion() { let map = PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769) + Collider(1): Collider(1), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769) ]) expectEqual(map._root.count, 3) expectEqual(map.reduce(0, { count, _ in count + 1 }), 3) @@ -201,190 +201,190 @@ final class CapsuleSmokeTests: CollectionTestCase { } func testCountForCopyOnWriteDeletion() { - var map: PersistentDictionary = [:] - - map[CollidableInt(32769)] = CollidableInt(32769) - map[CollidableInt(11, 1)] = CollidableInt(11, 1) - map[CollidableInt(12, 1)] = CollidableInt(12, 1) - map[CollidableInt(33, 33)] = CollidableInt(33, 33) - map[CollidableInt(11, 1)] = nil - map[CollidableInt(12, 1)] = nil + var map: PersistentDictionary = [:] + + map[Collider(32769)] = Collider(32769) + map[Collider(11, 1)] = Collider(11, 1) + map[Collider(12, 1)] = Collider(12, 1) + map[Collider(33, 33)] = Collider(33, 33) + map[Collider(11, 1)] = nil + map[Collider(12, 1)] = nil expectEqual(map._root.count, 2) expectEqual(map.reduce(0, { count, _ in count + 1 }), 2) map._root._invariantCheck() } func testCompactionWhenDeletingFromHashCollisionNode1() { - let map: PersistentDictionary = [:] + let map: PersistentDictionary = [:] var res1 = map - res1[CollidableInt(11, 1)] = CollidableInt(11, 1) - res1[CollidableInt(12, 1)] = CollidableInt(12, 1) + res1[Collider(11, 1)] = Collider(11, 1) + res1[Collider(12, 1)] = Collider(12, 1) - expectTrue(res1.contains(CollidableInt(11, 1))) - expectTrue(res1.contains(CollidableInt(12, 1))) + expectTrue(res1.contains(Collider(11, 1))) + expectTrue(res1.contains(Collider(12, 1))) expectEqual(res1.count, 2) expectEqual(PersistentDictionary([ - CollidableInt(11, 1): CollidableInt(11, 1), - CollidableInt(12, 1): CollidableInt(12, 1) + Collider(11, 1): Collider(11, 1), + Collider(12, 1): Collider(12, 1) ]), res1) var res2 = res1 - res2[CollidableInt(12, 1)] = nil + res2[Collider(12, 1)] = nil - expectTrue(res2.contains(CollidableInt(11, 1))) - expectFalse(res2.contains(CollidableInt(12, 1))) + expectTrue(res2.contains(Collider(11, 1))) + expectFalse(res2.contains(Collider(12, 1))) expectEqual(res2.count, 1) expectEqual( PersistentDictionary([ - CollidableInt(11, 1): CollidableInt(11, 1) + Collider(11, 1): Collider(11, 1) ]), res2) var res3 = res1 - res3[CollidableInt(11, 1)] = nil + res3[Collider(11, 1)] = nil - expectFalse(res3.contains(CollidableInt(11, 1))) - expectTrue(res3.contains(CollidableInt(12, 1))) + expectFalse(res3.contains(Collider(11, 1))) + expectTrue(res3.contains(Collider(12, 1))) expectEqual(res3.count, 1) expectEqual( - PersistentDictionary([CollidableInt(12, 1): CollidableInt(12, 1)]), + PersistentDictionary([Collider(12, 1): Collider(12, 1)]), res3) var resX = res1 - resX[CollidableInt(32769)] = CollidableInt(32769) - resX[CollidableInt(12, 1)] = nil + resX[Collider(32769)] = Collider(32769) + resX[Collider(12, 1)] = nil - expectTrue(resX.contains(CollidableInt(11, 1))) - expectFalse(resX.contains(CollidableInt(12, 1))) - expectTrue(resX.contains(CollidableInt(32769))) + expectTrue(resX.contains(Collider(11, 1))) + expectFalse(resX.contains(Collider(12, 1))) + expectTrue(resX.contains(Collider(32769))) expectEqual(resX.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(11, 1): CollidableInt(11, 1), - CollidableInt(32769): CollidableInt(32769)]), + Collider(11, 1): Collider(11, 1), + Collider(32769): Collider(32769)]), resX) var resY = res1 - resY[CollidableInt(32769)] = CollidableInt(32769) - resY[CollidableInt(32769)] = nil + resY[Collider(32769)] = Collider(32769) + resY[Collider(32769)] = nil - expectTrue(resY.contains(CollidableInt(11, 1))) - expectTrue(resY.contains(CollidableInt(12, 1))) - expectFalse(resY.contains(CollidableInt(32769))) + expectTrue(resY.contains(Collider(11, 1))) + expectTrue(resY.contains(Collider(12, 1))) + expectFalse(resY.contains(Collider(32769))) expectEqual(resY.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(11, 1): CollidableInt(11, 1), - CollidableInt(12, 1): CollidableInt(12, 1)]), + Collider(11, 1): Collider(11, 1), + Collider(12, 1): Collider(12, 1)]), resY) } func testCompactionWhenDeletingFromHashCollisionNode2() { - let map: PersistentDictionary = [:] + let map: PersistentDictionary = [:] var res1 = map - res1[CollidableInt(32769_1, 32769)] = CollidableInt(32769_1, 32769) - res1[CollidableInt(32769_2, 32769)] = CollidableInt(32769_2, 32769) + res1[Collider(32769_1, 32769)] = Collider(32769_1, 32769) + res1[Collider(32769_2, 32769)] = Collider(32769_2, 32769) - expectTrue(res1.contains(CollidableInt(32769_1, 32769))) - expectTrue(res1.contains(CollidableInt(32769_2, 32769))) + expectTrue(res1.contains(Collider(32769_1, 32769))) + expectTrue(res1.contains(Collider(32769_2, 32769))) expectEqual(res1.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res1) var res2 = res1 - res2[CollidableInt(1)] = CollidableInt(1) + res2[Collider(1)] = Collider(1) - expectTrue(res2.contains(CollidableInt(1))) - expectTrue(res2.contains(CollidableInt(32769_1, 32769))) - expectTrue(res2.contains(CollidableInt(32769_2, 32769))) + expectTrue(res2.contains(Collider(1))) + expectTrue(res2.contains(Collider(32769_1, 32769))) + expectTrue(res2.contains(Collider(32769_2, 32769))) expectEqual(res2.count, 3) expectEqual( PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(1): Collider(1), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res2) var res3 = res2 - res3[CollidableInt(32769_2, 32769)] = nil + res3[Collider(32769_2, 32769)] = nil - expectTrue(res3.contains(CollidableInt(1))) - expectTrue(res3.contains(CollidableInt(32769_1, 32769))) + expectTrue(res3.contains(Collider(1))) + expectTrue(res3.contains(Collider(32769_1, 32769))) expectEqual(res3.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769)]), + Collider(1): Collider(1), + Collider(32769_1, 32769): Collider(32769_1, 32769)]), res3) } func testCompactionWhenDeletingFromHashCollisionNode3() { - let map: PersistentDictionary = [:] + let map: PersistentDictionary = [:] var res1 = map - res1[CollidableInt(32769_1, 32769)] = CollidableInt(32769_1, 32769) - res1[CollidableInt(32769_2, 32769)] = CollidableInt(32769_2, 32769) + res1[Collider(32769_1, 32769)] = Collider(32769_1, 32769) + res1[Collider(32769_2, 32769)] = Collider(32769_2, 32769) - expectTrue(res1.contains(CollidableInt(32769_1, 32769))) - expectTrue(res1.contains(CollidableInt(32769_2, 32769))) + expectTrue(res1.contains(Collider(32769_1, 32769))) + expectTrue(res1.contains(Collider(32769_2, 32769))) expectEqual(res1.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res1) var res2 = res1 - res2[CollidableInt(1)] = CollidableInt(1) + res2[Collider(1)] = Collider(1) - expectTrue(res2.contains(CollidableInt(1))) - expectTrue(res2.contains(CollidableInt(32769_1, 32769))) - expectTrue(res2.contains(CollidableInt(32769_2, 32769))) + expectTrue(res2.contains(Collider(1))) + expectTrue(res2.contains(Collider(32769_1, 32769))) + expectTrue(res2.contains(Collider(32769_2, 32769))) expectEqual(res2.count, 3) expectEqual( PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(1): Collider(1), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res2) var res3 = res2 - res3[CollidableInt(1)] = nil + res3[Collider(1)] = nil - expectTrue(res3.contains(CollidableInt(32769_1, 32769))) - expectTrue(res3.contains(CollidableInt(32769_2, 32769))) + expectTrue(res3.contains(Collider(32769_1, 32769))) + expectTrue(res3.contains(Collider(32769_2, 32769))) expectEqual(res3.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res3) @@ -392,51 +392,51 @@ final class CapsuleSmokeTests: CollectionTestCase { } func testCompactionWhenDeletingFromHashCollisionNode4() { - let map: PersistentDictionary = [:] + let map: PersistentDictionary = [:] var res1 = map - res1[CollidableInt(32769_1, 32769)] = CollidableInt(32769_1, 32769) - res1[CollidableInt(32769_2, 32769)] = CollidableInt(32769_2, 32769) + res1[Collider(32769_1, 32769)] = Collider(32769_1, 32769) + res1[Collider(32769_2, 32769)] = Collider(32769_2, 32769) - expectTrue(res1.contains(CollidableInt(32769_1, 32769))) - expectTrue(res1.contains(CollidableInt(32769_2, 32769))) + expectTrue(res1.contains(Collider(32769_1, 32769))) + expectTrue(res1.contains(Collider(32769_2, 32769))) expectEqual(res1.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res1) var res2 = res1 - res2[CollidableInt(5)] = CollidableInt(5) + res2[Collider(5)] = Collider(5) - expectTrue(res2.contains(CollidableInt(5))) - expectTrue(res2.contains(CollidableInt(32769_1, 32769))) - expectTrue(res2.contains(CollidableInt(32769_2, 32769))) + expectTrue(res2.contains(Collider(5))) + expectTrue(res2.contains(Collider(32769_1, 32769))) + expectTrue(res2.contains(Collider(32769_2, 32769))) expectEqual(res2.count, 3) expectEqual( PersistentDictionary([ - CollidableInt(5): CollidableInt(5), - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(5): Collider(5), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res2) var res3 = res2 - res3[CollidableInt(5)] = nil + res3[Collider(5)] = nil - expectTrue(res3.contains(CollidableInt(32769_1, 32769))) - expectTrue(res3.contains(CollidableInt(32769_2, 32769))) + expectTrue(res3.contains(Collider(32769_1, 32769))) + expectTrue(res3.contains(Collider(32769_2, 32769))) expectEqual(res3.count, 2) expectEqual( PersistentDictionary([ - CollidableInt(32769_1, 32769): CollidableInt(32769_1, 32769), - CollidableInt(32769_2, 32769): CollidableInt(32769_2, 32769)]), + Collider(32769_1, 32769): Collider(32769_1, 32769), + Collider(32769_2, 32769): Collider(32769_2, 32769)]), res3) @@ -444,44 +444,44 @@ final class CapsuleSmokeTests: CollectionTestCase { } func testCompactionWhenDeletingFromHashCollisionNode5() { - let map: PersistentDictionary = [:] + let map: PersistentDictionary = [:] var res1 = map - res1[CollidableInt(1)] = CollidableInt(1) - res1[CollidableInt(1026)] = CollidableInt(1026) - res1[CollidableInt(32770_1, 32770)] = CollidableInt(32770_1, 32770) - res1[CollidableInt(32770_2, 32770)] = CollidableInt(32770_2, 32770) + res1[Collider(1)] = Collider(1) + res1[Collider(1026)] = Collider(1026) + res1[Collider(32770_1, 32770)] = Collider(32770_1, 32770) + res1[Collider(32770_2, 32770)] = Collider(32770_2, 32770) - expectTrue(res1.contains(CollidableInt(1))) - expectTrue(res1.contains(CollidableInt(1026))) - expectTrue(res1.contains(CollidableInt(32770_1, 32770))) - expectTrue(res1.contains(CollidableInt(32770_2, 32770))) + expectTrue(res1.contains(Collider(1))) + expectTrue(res1.contains(Collider(1026))) + expectTrue(res1.contains(Collider(32770_1, 32770))) + expectTrue(res1.contains(Collider(32770_2, 32770))) expectEqual(res1.count, 4) expectEqual( PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(1026): CollidableInt(1026), - CollidableInt(32770_1, 32770): CollidableInt(32770_1, 32770), - CollidableInt(32770_2, 32770): CollidableInt(32770_2, 32770)]), + Collider(1): Collider(1), + Collider(1026): Collider(1026), + Collider(32770_1, 32770): Collider(32770_1, 32770), + Collider(32770_2, 32770): Collider(32770_2, 32770)]), res1) var res2 = res1 - res2[CollidableInt(1026)] = nil + res2[Collider(1026)] = nil - expectTrue(res2.contains(CollidableInt(1))) - expectFalse(res2.contains(CollidableInt(1026))) - expectTrue(res2.contains(CollidableInt(32770_1, 32770))) - expectTrue(res2.contains(CollidableInt(32770_2, 32770))) + expectTrue(res2.contains(Collider(1))) + expectFalse(res2.contains(Collider(1026))) + expectTrue(res2.contains(Collider(32770_1, 32770))) + expectTrue(res2.contains(Collider(32770_2, 32770))) expectEqual(res2.count, 3) expectEqual( PersistentDictionary([ - CollidableInt(1): CollidableInt(1), - CollidableInt(32770_1, 32770): CollidableInt(32770_1, 32770), - CollidableInt(32770_2, 32770): CollidableInt(32770_2, 32770)]), + Collider(1): Collider(1), + Collider(32770_1, 32770): Collider(32770_1, 32770), + Collider(32770_2, 32770): Collider(32770_2, 32770)]), res2) } @@ -499,19 +499,19 @@ final class CapsuleSmokeTests: CollectionTestCase { let upperBound = 1_000 // '+' prefixed values - var map1: PersistentDictionary = [:] + var map1: PersistentDictionary = [:] for index in 0.. = map1 + var map2: PersistentDictionary = map1 for index in 0.. = map1 + var map3: PersistentDictionary = map1 for index in 0.. = [ - CollidableInt(11, 1): "a", - CollidableInt(12, 1): "a", - CollidableInt(32769): "b" + let map1: PersistentDictionary = [ + Collider(11, 1): "a", + Collider(12, 1): "a", + Collider(32769): "b" ] - var map2: PersistentDictionary = [:] + var map2: PersistentDictionary = [:] for (key, value) in map1 { map2[key] = value } @@ -573,21 +573,22 @@ final class CapsuleSmokeTests: CollectionTestCase { } func test_indexForKey_hashCollision() { - let a = CollidableInt(1, "1000") - let b = CollidableInt(2, "1000") - let c = CollidableInt(3, "1001") - let map: PersistentDictionary = [ + let a = Collider(1, "1000") + let b = Collider(2, "1000") + let c = Collider(3, "1001") + let map: PersistentDictionary = [ a: "a", b: "b", c: "c", ] + let indices = Array(map.indices) - typealias Index = PersistentDictionary.Index - expectEqual(map.index(forKey: a), Index(_value: 2)) - expectEqual(map.index(forKey: b), Index(_value: 1)) - expectEqual(map.index(forKey: c), Index(_value: 0)) - expectNil(map.index(forKey: CollidableInt(4, "1000"))) + typealias Index = PersistentDictionary.Index + expectEqual(map.index(forKey: a), indices[1]) + expectEqual(map.index(forKey: b), indices[2]) + expectEqual(map.index(forKey: c), indices[0]) + expectNil(map.index(forKey: Collider(4, "1000"))) } func test_indexForKey() { @@ -605,23 +606,22 @@ final class CapsuleSmokeTests: CollectionTestCase { } func test_indexForKey_exhaustIndices() { - var map: PersistentDictionary = [:] + var map: PersistentDictionary = [:] let range = 0 ..< 10_000 for value in range { - map[CollidableInt(value)] = value + map[Collider(value)] = value } - var expectedPositions = Set(range) + var expectedPositions = Set(map.indices) for expectedValue in range { - let position = map.index(forKey: CollidableInt(expectedValue))! - let actualValue = map[position].value - - expectEqual(expectedValue, actualValue) - - expectedPositions.remove(position._value) + expectNotNil(map.index(forKey: Collider(expectedValue))) { index in + let actualValue = map[index].value + expectEqual(expectedValue, actualValue) + expectedPositions.remove(index) + } } expectTrue(expectedPositions.isEmpty) diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift b/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift index 2800d690d..e6fdcb426 100644 --- a/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift +++ b/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift @@ -17,6 +17,250 @@ class PersistentDictionaryTests: CollectionTestCase { let d = PersistentDictionary() expectEqualElements(d, []) expectEqual(d.count, 0) + + var it = d.makeIterator() + expectNil(it.next()) + expectNil(it.next()) + expectNil(it.next()) + + expectEqual(d.startIndex, d.endIndex) + + expectEqual(d.distance(from: d.startIndex, to: d.endIndex), 0) + } + + func test_remove_update_basics() throws { + var d = PersistentDictionary() + + d.updateValue(1, forKey: "One") + d.updateValue(2, forKey: "Two") + d.updateValue(3, forKey: "Three") + + expectEqual(d.count, 3) + expectEqual(d["One"], 1) + expectEqual(d["Two"], 2) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + expectEqual(d.removeValue(forKey: "Two"), 2) + + expectEqual(d.count, 2) + expectEqual(d["One"], 1) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + expectEqual(d.removeValue(forKey: "Two"), nil) + expectEqual(d.removeValue(forKey: "One"), 1) + + expectEqual(d.count, 1) + expectEqual(d["One"], nil) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + expectEqual(d.removeValue(forKey: "One"), nil) + expectEqual(d.removeValue(forKey: "Two"), nil) + expectEqual(d.removeValue(forKey: "Three"), 3) + + expectEqual(d.count, 0) + expectEqual(d["One"], nil) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], nil) + expectEqual(d["Four"], nil) + } + + func test_subscript_setter_basics() throws { + var d = PersistentDictionary() + + d["One"] = 1 + d["Two"] = 2 + d["Three"] = 3 + + expectEqual(d.count, 3) + expectEqual(d["One"], 1) + expectEqual(d["Two"], 2) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + d["Two"] = nil + + expectEqual(d.count, 2) + expectEqual(d["One"], 1) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + d["Two"] = nil + d["One"] = nil + + expectEqual(d.count, 1) + expectEqual(d["One"], nil) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], 3) + expectEqual(d["Four"], nil) + + d["One"] = nil + d["Two"] = nil + d["Three"] = nil + + expectEqual(d.count, 0) + expectEqual(d["One"], nil) + expectEqual(d["Two"], nil) + expectEqual(d["Three"], nil) + expectEqual(d["Four"], nil) + } + + func test_add_remove() throws { + var d = PersistentDictionary() + + let c = 400 + for i in 0 ..< c { + expectNil(d.updateValue(i, forKey: "\(i)")) + expectEqual(d.count, i + 1) + } + + for i in 0 ..< c { + expectEqual(d["\(i)"], i) + } + + for i in 0 ..< c { + expectEqual(d.updateValue(2 * i, forKey: "\(i)"), i) + expectEqual(d.count, c) + } + + for i in 0 ..< c { + expectEqual(d["\(i)"], 2 * i) + } + + var remaining = c + for i in 0 ..< c { + expectEqual(d.removeValue(forKey: "\(i)"), 2 * i) + remaining -= 1 + expectEqual(d.count, remaining) + } + + } + + func test_collisions() throws { + var d = PersistentDictionary() + + let count = 100 + let groups = 20 + + for i in 0 ..< count { + let h = i % groups + let key = Collider(i, Hash(h)) + expectEqual(d[key], nil) + expectNil(d.updateValue(i, forKey: key)) + expectEqual(d[key], i) + } + + for i in 0 ..< count { + let h = i % groups + let key = Collider(i, Hash(h)) + expectEqual(d[key], i) + expectEqual(d.updateValue(2 * i, forKey: key), i) + expectEqual(d[key], 2 * i) + } + + for i in 0 ..< count { + let h = i % groups + let key = Collider(i, Hash(h)) + expectEqual(d[key], 2 * i) + expectEqual(d.removeValue(forKey: key), 2 * i) + expectEqual(d[key], nil) + } + } + + func test_shared_copies() throws { + var d = PersistentDictionary() + + let c = 200 + for i in 0 ..< c { + expectNil(d.updateValue(i, forKey: i)) + } + + let copy = d + for i in 0 ..< c { + expectEqual(d.updateValue(2 * i, forKey: i), i) + } + + for i in 0 ..< c { + expectEqual(copy[i], i) + } + + let copy2 = d + for i in 0 ..< c { + expectEqual(d.removeValue(forKey: i), 2 * i) + } + + for i in 0 ..< c { + expectEqual(copy2[i], 2 * i) + } + } + + func test_Sequence_basic() { + var d: PersistentDictionary = [1: 2] + var it = d.makeIterator() + expectEquivalent(it.next(), (1, 2), by: { $0 == $1 }) + expectNil(it.next()) + expectNil(it.next()) + + d[1] = nil + it = d.makeIterator() + expectNil(it.next()) + expectNil(it.next()) + } + + func test_Sequence_400() { + var d = PersistentDictionary() + let c = 400 + for i in 0 ..< c { + expectNil(d.updateValue(i, forKey: i)) + } + + var seen: Set = [] + for (key, value) in d { + expectEqual(key, value) + expectTrue(seen.insert(key).inserted, "Duplicate key seen: \(key)") + } + expectEqual(seen.count, c) + expectTrue(seen.isSuperset(of: 0 ..< c)) + } + + func test_Sequence_collisions() { + var d = PersistentDictionary() + + let count = 100 + let groups = 20 + + for i in 0 ..< count { + let h = i % groups + let key = Collider(i, Hash(h)) + expectNil(d.updateValue(i, forKey: key)) + } + + var seen: Set = [] + for (key, value) in d { + expectEqual(key.identity, value) + expectTrue(seen.insert(key.identity).inserted, "Duplicate key: \(key)") + } + expectEqual(seen.count, count) + expectTrue(seen.isSuperset(of: 0 ..< count)) + } + + func test_BidirectionalCollection_fixtures() { + withEachFixture { fixture in + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(for: fixture.items) + checkBidirectionalCollection(d, expectedContents: ref, by: ==) + } + } + } + + func test_BidirectionalCollection_random() { + let d = PersistentDictionary(uniqueKeys: 0 ..< 100, values: 0 ..< 100) + checkBidirectionalCollection(d, expectedContents: Array(d), by: ==) } // func test_uniqueKeysWithValues_Dictionary() { @@ -179,6 +423,7 @@ class PersistentDictionaryTests: CollectionTestCase { } } + #if false // TODO: determine how to best calculate the expected order of the hash tree // for testing purposes, without relying on the actual implementation func test_index_forKey() { @@ -196,6 +441,7 @@ class PersistentDictionaryTests: CollectionTestCase { } } } + #endif #if false // TODO: determine how to best calculate the expected order of the hash tree @@ -904,18 +1150,18 @@ class PersistentDictionaryTests: CollectionTestCase { // } func test_CustomStringConvertible() { - let a: PersistentDictionary = [:] + let a: PersistentDictionary = [:] expectEqual(a.description, "[:]") - let b: PersistentDictionary = [ - CollidableInt(0): 1 + let b: PersistentDictionary = [ + RawCollider(0): 1 ] expectEqual(b.description, "[0: 1]") - let c: PersistentDictionary = [ - CollidableInt(0): 1, - CollidableInt(2): 3, - CollidableInt(4): 5, + let c: PersistentDictionary = [ + RawCollider(0): 1, + RawCollider(2): 3, + RawCollider(4): 5, ] expectEqual(c.description, "[0: 1, 2: 3, 4: 5]") } diff --git a/Tests/PersistentCollectionsTests/Utilities.swift b/Tests/PersistentCollectionsTests/Utilities.swift new file mode 100644 index 000000000..b4e6ad0bf --- /dev/null +++ b/Tests/PersistentCollectionsTests/Utilities.swift @@ -0,0 +1,31 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + +import _CollectionsTestSupport +import PersistentCollections + +extension LifetimeTracker { + func persistentDictionary( + keys: Keys + ) -> ( + dictionary: PersistentDictionary, LifetimeTracked>, + keys: [LifetimeTracked], + values: [LifetimeTracked] + ) + where Keys.Element == Int + { + let k = Array(keys) + let keys = self.instances(for: k) + let values = self.instances(for: k.map { $0 + 100 }) + let dictionary = PersistentDictionary(uniqueKeys: keys, values: values) + return (dictionary, keys, values) + } +} diff --git a/Tests/PersistentCollectionsTests/_CollidableInt.swift b/Tests/PersistentCollectionsTests/_CollidableInt.swift deleted file mode 100644 index 0e6ae5cd6..000000000 --- a/Tests/PersistentCollectionsTests/_CollidableInt.swift +++ /dev/null @@ -1,69 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// This source file is part of the Swift Collections open source project -// -// Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// -//===----------------------------------------------------------------------===// - -extension Int { - public init(hashString: String) { - let s = String(hashString.reversed()) - let hash = UInt(s, radix: 32)! - self = Int(bitPattern: hash) - } -} - -public final class CollidableInt: - CustomStringConvertible, CustomDebugStringConvertible, Equatable, Hashable -{ - public let value: Int - let _hashValue: Int - - public init(_ value: Int) { - self.value = value - self._hashValue = value - } - - public init(_ value: Int, _ hashValue: Int) { - self.value = value - self._hashValue = hashValue - } - - public init(_ value: Int, _ hashString: String) { - - self.value = value - self._hashValue = Int(hashString: hashString) - } - - public var description: String { - return "\(value)" - } - - public var debugDescription: String { - return "\(value) [hash = \(_hashValue)]" - } - - public func _rawHashValue(seed: Int) -> Int { - _hashValue - } - - public func hash(into hasher: inout Hasher) { - fatalError() - } - - public var hashValue: Int { - fatalError() - } - - public static func == (lhs: CollidableInt, rhs: CollidableInt) -> Bool { - if lhs.value == rhs.value { - precondition(lhs._hashValue == rhs._hashValue) - return true - } - return false - } -} From 0fe7f080c01bb75aa535aa34fd83d01ad05c46c6 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Fri, 16 Sep 2022 01:30:06 -0700 Subject: [PATCH 09/18] =?UTF-8?q?[PersistentCollections]=20offset=20?= =?UTF-8?q?=E2=86=92=20slot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Internally) standardize on using the word “slot” to refer to an id for a location within a contiguous storage buffer that can hold a single item of some kind. “Offset” is sometimes used in public APIs (to refer to integer distances within collections, relative to some start position), and using the same term here causes internal naming difficulties. A “bucket” is therefore a more specific name for a slot within a hash table. --- .../Node/_AncestorOffsets.swift | 22 +- .../PersistentCollections/Node/_Bitmap.swift | 8 +- .../Node/_Node+Debugging.swift | 32 +-- .../Node/_Node+Initializers.swift | 6 +- .../Node/_Node+Invariants.swift | 18 +- .../Node/_Node+Lookups.swift | 67 +++--- .../Node/_Node+Mutations.swift | 122 +++++----- .../Node/_Node+Storage.swift | 2 + .../Node/_Node+UnsafeHandle.swift | 36 +-- .../PersistentCollections/Node/_Node.swift | 3 +- .../Node/_RawNode+UnsafeHandle.swift | 15 +- .../PersistentCollections/Node/_RawNode.swift | 10 +- .../PersistentCollections/Node/_Slot.swift | 104 +++++++++ .../Node/_StorageHeader.swift | 10 + .../Node/_UnmanagedNode.swift | 16 +- .../Node/_UnsafePath.swift | 210 +++++++++--------- .../PersistentDictionary+Collection.swift | 2 +- 17 files changed, 414 insertions(+), 269 deletions(-) create mode 100644 Sources/PersistentCollections/Node/_Slot.swift diff --git a/Sources/PersistentCollections/Node/_AncestorOffsets.swift b/Sources/PersistentCollections/Node/_AncestorOffsets.swift index b9b5257a5..297fbb772 100644 --- a/Sources/PersistentCollections/Node/_AncestorOffsets.swift +++ b/Sources/PersistentCollections/Node/_AncestorOffsets.swift @@ -11,7 +11,7 @@ @usableFromInline @frozen -struct _AncestorOffsets { +struct _AncestorSlots { @usableFromInline internal var path: UInt @@ -21,37 +21,37 @@ struct _AncestorOffsets { } } -extension _AncestorOffsets: Equatable { +extension _AncestorSlots: Equatable { @inlinable @inline(__always) internal static func ==(left: Self, right: Self) -> Bool { left.path == right.path } } -extension _AncestorOffsets { +extension _AncestorSlots { @inlinable @inline(__always) internal static var empty: Self { Self(0) } } -extension _AncestorOffsets { +extension _AncestorSlots { @inlinable @inline(__always) - internal subscript(_ level: _Level) -> Int { + internal subscript(_ level: _Level) -> _Slot { get { assert(level.shift < UInt.bitWidth) - return Int(bitPattern: (path &>> level.shift) & _Bucket.bitMask) + return _Slot((path &>> level.shift) & _Bucket.bitMask) } set { - assert(newValue < UInt.bitWidth) - assert(self[level] == 0) - path |= (UInt(bitPattern: newValue) &<< level.shift) + assert(newValue._value < UInt.bitWidth) + assert(self[level] == .zero) + path |= (UInt(truncatingIfNeeded: newValue._value) &<< level.shift) } } @inlinable - internal func truncating(to level: _Level) -> _AncestorOffsets { + internal func truncating(to level: _Level) -> _AncestorSlots { assert(level.shift <= UInt.bitWidth) guard level.shift < UInt.bitWidth else { return self } - return _AncestorOffsets(path & ((1 &<< level.shift) &- 1)) + return _AncestorSlots(path & ((1 &<< level.shift) &- 1)) } @inlinable diff --git a/Sources/PersistentCollections/Node/_Bitmap.swift b/Sources/PersistentCollections/Node/_Bitmap.swift index 4ab3311dd..946cba232 100644 --- a/Sources/PersistentCollections/Node/_Bitmap.swift +++ b/Sources/PersistentCollections/Node/_Bitmap.swift @@ -105,13 +105,13 @@ extension _Bitmap { } @inlinable @inline(__always) - internal func offset(of bucket: _Bucket) -> Int { - _value._rank(ofBit: bucket.value) + internal func slot(of bucket: _Bucket) -> _Slot { + _Slot(_value._rank(ofBit: bucket.value)) } @inlinable @inline(__always) - internal func bucket(at offset: Int) -> _Bucket { - _Bucket(_value._bit(ranked: offset)!) + internal func bucket(at slot: _Slot) -> _Bucket { + _Bucket(_value._bit(ranked: slot.value)!) } } diff --git a/Sources/PersistentCollections/Node/_Node+Debugging.swift b/Sources/PersistentCollections/Node/_Node+Debugging.swift index 053a8e3b2..1cae50d49 100644 --- a/Sources/PersistentCollections/Node/_Node+Debugging.swift +++ b/Sources/PersistentCollections/Node/_Node+Debugging.swift @@ -40,8 +40,8 @@ extension _Node.Storage { } extension _Node.UnsafeHandle { - internal func _itemString(at offset: Int) -> String { - let item = self[item: offset] + internal func _itemString(at slot: _Slot) -> String { + let item = self[item: slot] let hash = _Hash(item.key).description return "hash: \(hash), key: \(item.key), value: \(item.value)" } @@ -73,39 +73,39 @@ extension _Node.UnsafeHandle { """) guard limit > 0 else { return } if iterationOrder { - for offset in 0 ..< itemCount { - print(" \(restPrefix)[\(offset)] \(_itemString(at: offset))") + for slot in stride(from: .zero, to: itemEnd, by: 1) { + print(" \(restPrefix)[\(slot)] \(_itemString(at: slot))") } - for offset in 0 ..< childCount { - self[child: offset].dump( + for slot in stride(from: .zero, to: childEnd, by: 1) { + self[child: slot].dump( iterationOrder: true, limit: limit - 1, - firstPrefix: " \(restPrefix).\(offset)", - restPrefix: " \(restPrefix).\(offset)", + firstPrefix: " \(restPrefix).\(slot)", + restPrefix: " \(restPrefix).\(slot)", depth: depth + 1) } } else if isCollisionNode { - for offset in 0 ..< itemCount { - print("\(restPrefix)[\(offset)] \(_itemString(at: offset))") + for slot in stride(from: .zero, to: itemEnd, by: 1) { + print("\(restPrefix)[\(slot)] \(_itemString(at: slot))") } } else { - var itemOffset = 0 - var childOffset = 0 + var itemSlot: _Slot = .zero + var childSlot: _Slot = .zero for b in 0 ..< UInt(_Bitmap.capacity) { let bucket = _Bucket(b) let bucketStr = "#\(String(b, radix: _Bitmap.capacity, uppercase: true))" if itemMap.contains(bucket) { - print("\(restPrefix) \(bucketStr) \(_itemString(at: itemOffset))") - itemOffset += 1 + print("\(restPrefix) \(bucketStr) \(_itemString(at: itemSlot))") + itemSlot = itemSlot.next() } else if childMap.contains(bucket) { - self[child: childOffset].dump( + self[child: childSlot].dump( iterationOrder: false, limit: limit - 1, firstPrefix: "\(restPrefix) \(bucketStr) ", restPrefix: "\(restPrefix) ", depth: depth + 1) - childOffset += 1 + childSlot = childSlot.next() } } } diff --git a/Sources/PersistentCollections/Node/_Node+Initializers.swift b/Sources/PersistentCollections/Node/_Node+Initializers.swift index 86ba091c2..b783ecb34 100644 --- a/Sources/PersistentCollections/Node/_Node+Initializers.swift +++ b/Sources/PersistentCollections/Node/_Node+Initializers.swift @@ -54,7 +54,7 @@ extension _Node { update { $0.childMap.insert(bucket) $0.bytesFree &-= MemoryLayout<_Node>.stride - $0.childPtr(at: 0).initialize(to: _child) + $0.childPtr(at: .zero).initialize(to: _child) } } @@ -71,8 +71,8 @@ extension _Node { $0.itemMap.insert(itemBucket) $0.childMap.insert(childBucket) $0.bytesFree &-= MemoryLayout.stride + MemoryLayout<_Node>.stride - $0.itemPtr(at: 0).initialize(to: _item) - $0.childPtr(at: 0).initialize(to: child) + $0.itemPtr(at: .zero).initialize(to: _item) + $0.childPtr(at: .zero).initialize(to: child) } } } diff --git a/Sources/PersistentCollections/Node/_Node+Invariants.swift b/Sources/PersistentCollections/Node/_Node+Invariants.swift index 06eb9aa0f..df0fc53bd 100644 --- a/Sources/PersistentCollections/Node/_Node+Invariants.swift +++ b/Sources/PersistentCollections/Node/_Node+Invariants.swift @@ -31,7 +31,7 @@ extension _Node { #if COLLECTIONS_INTERNAL_CHECKS @usableFromInline @inline(never) internal func _invariantCheck() { - storage.header._invariantCheck() + raw.storage.header._invariantCheck() read { let itemBytes = $0.itemCount * MemoryLayout.stride let childBytes = $0.childCount * MemoryLayout<_Node>.stride @@ -55,31 +55,31 @@ extension _Node { if $0.isCollisionNode { precondition(count == $0.itemCount) precondition(count > 0) - let key = $0[item: 0].key + let key = $0[item: .zero].key let hash = _Hash(key) precondition( hash.isEqual(to: path, upTo: level), "Misplaced colliding key '\(key)': \(path) isn't a prefix of \(hash)") - for item in $0._items.dropFirst() { + for item in $0.reverseItems.dropFirst() { precondition(_Hash(item.key) == hash) } } - var itemOffset = 0 - var childOffset = 0 + var itemSlot: _Slot = .zero + var childSlot: _Slot = .zero for b in 0 ..< UInt(_Bitmap.capacity) { let bucket = _Bucket(b) let path = path.appending(bucket, at: level) if $0.itemMap.contains(bucket) { - let key = $0._items[itemOffset].key + let key = $0[item: itemSlot].key let hash = _Hash(key) precondition( hash.isEqual(to: path, upTo: level.descend()), "Misplaced key '\(key)': \(path) isn't a prefix of \(hash)") - itemOffset += 1 + itemSlot = itemSlot.next() } if $0.hasChildren && $0.childMap.contains(bucket) { - $0[child: childOffset]._fullInvariantCheck(level.descend(), path) - childOffset += 1 + $0[child: childSlot]._fullInvariantCheck(level.descend(), path) + childSlot = childSlot.next() } } } diff --git a/Sources/PersistentCollections/Node/_Node+Lookups.swift b/Sources/PersistentCollections/Node/_Node+Lookups.swift index 182f7d27d..112d32f2b 100644 --- a/Sources/PersistentCollections/Node/_Node+Lookups.swift +++ b/Sources/PersistentCollections/Node/_Node+Lookups.swift @@ -23,30 +23,30 @@ @frozen internal enum _FindResult { /// The item we're looking for is stored directly in this node, at the - /// bucket / item offset identified in the payload. + /// bucket / item slot identified in the payload. /// /// If the current node is a collision node, then the bucket value is /// set to `_Bucket.invalid`. - case found(_Bucket, Int) + case found(_Bucket, _Slot) /// The item we're looking for is not currently inside the subtree rooted at /// this node. /// /// If we wanted to insert it, then its correct slot is within this node - /// at the specified bucket / item offset. (Which is currently empty.) + /// at the specified bucket / item slot. (Which is currently empty.) /// /// If the current node is a collision node, then the bucket value is /// set to `_Bucket.invalid`. - case notFound(_Bucket, Int) + case notFound(_Bucket, _Slot) /// The item we're looking for is not currently inside the subtree rooted at /// this node. /// /// If we wanted to insert it, then it would need to be stored in this node - /// at the specified bucket / item offset. However, that bucket is already + /// at the specified bucket / item slot. However, that bucket is already /// occupied by another item, so the insertion would need to involve replacing /// it with a new child node. /// /// (This case is never returned if the current node is a collision node.) - case newCollision(_Bucket, Int) + case newCollision(_Bucket, _Slot) /// The item we're looking for is not in this subtree. /// /// However, the item doesn't belong in this subtree at all. This is an @@ -67,10 +67,10 @@ internal enum _FindResult { case expansion(_Hash) /// The item we're looking for is not directly stored in this node, but it /// might be somewhere in the subtree rooted at the child at the given - /// bucket & offset. + /// bucket & slot. /// /// (This case is never returned if the current node is a collision node.) - case descend(_Bucket, Int) + case descend(_Bucket, _Slot) } extension _Node { @@ -89,32 +89,32 @@ extension _Node.UnsafeHandle { ) -> _FindResult { guard !isCollisionNode else { if !level.isAtBottom { - let h = _Hash(self[item: 0].key) + let h = _Hash(self[item: .zero].key) if h != hash { return .expansion(h) } } // Note: this searches the items in reverse insertion order. - guard let offset = reverseItems.firstIndex(where: { $0.key == key }) else { - return .notFound(.invalid, itemCount) + guard let slot = reverseItems.firstIndex(where: { $0.key == key }) else { + return .notFound(.invalid, itemEnd) } - return .found(.invalid, itemCount &- 1 &- offset) + return .found(.invalid, _Slot(itemCount &- 1 &- slot)) } let bucket = hash[level] if itemMap.contains(bucket) { - let offset = itemMap.offset(of: bucket) - if self[item: offset].key == key { - return .found(bucket, offset) + let slot = itemMap.slot(of: bucket) + if self[item: slot].key == key { + return .found(bucket, slot) } - return .newCollision(bucket, offset) + return .newCollision(bucket, slot) } if childMap.contains(bucket) { - let offset = childMap.offset(of: bucket) - return .descend(bucket, offset) + let slot = childMap.slot(of: bucket) + return .descend(bucket, slot) } - // Don't calculate the offset unless the caller will need it. - let offset = forInsert ? itemMap.offset(of: bucket) : 0 - return .notFound(bucket, offset) + // Don't calculate the slot unless the caller will need it. + let slot = forInsert ? itemMap.slot(of: bucket) : .zero + return .notFound(bucket, slot) } } @@ -126,12 +126,12 @@ extension _Node { read { let r = $0.find(level, key, hash, forInsert: false) switch r { - case .found(_, let offset): - return $0[item: offset].value + case .found(_, let slot): + return $0[item: slot].value case .notFound, .newCollision, .expansion: return nil - case .descend(_, let offset): - return $0[child: offset].get(level.descend(), key, hash) + case .descend(_, let slot): + return $0[child: slot].get(level.descend(), key, hash) } } } @@ -147,8 +147,8 @@ extension _Node { return true case .notFound, .newCollision, .expansion: return false - case .descend(_, let offset): - return $0[child: offset].containsKey(level.descend(), key, hash) + case .descend(_, let slot): + return $0[child: slot].containsKey(level.descend(), key, hash) } } } @@ -161,17 +161,18 @@ extension _Node { ) -> Int? { let r = find(level, key, hash, forInsert: false) switch r { - case .found(_, let offset): - return offset + case .found(_, let slot): + return slot.value case .notFound, .newCollision, .expansion: return nil - case .descend(_, let offset): + case .descend(_, let slot): return read { h in let children = h._children - let p = children[offset].position(forKey: key, level.descend(), hash) + let p = children[slot.value] + .position(forKey: key, level.descend(), hash) guard let p = p else { return nil } let c = h.itemCount &+ p - return children[..= 0 && offset <= c) + assert(slot.value <= c) let stride = MemoryLayout.stride assert(bytesFree >= stride) @@ -31,44 +31,44 @@ extension _Node.UnsafeHandle { .advanced(by: byteCapacity &- (c &+ 1) &* stride) .bindMemory(to: Element.self, capacity: 1) - let prefix = c &- offset + let prefix = c &- slot.value start.moveInitialize(from: start + 1, count: prefix) (start + prefix).initialize(to: item) } - /// Insert `child` at `offset`. There must be enough free space in the node + /// Insert `child` at `slot`. There must be enough free space in the node /// to fit the new child. /// /// `childMap` must not yet reflect the insertion at the time this /// function is called. This method does not update `childMap`. @inlinable - internal func _insertChild(_ child: __owned _Node, at offset: Int) { + internal func _insertChild(_ child: __owned _Node, at slot: _Slot) { assertMutable() assert(!isCollisionNode) let c = childMap.count - assert(offset >= 0 && offset <= c) + assert(slot.value <= c) let stride = MemoryLayout<_Node>.stride assert(bytesFree >= stride) bytesFree &-= stride _memory.bindMemory(to: _Node.self, capacity: c &+ 1) - let q = _childrenStart + offset - (q + 1).moveInitialize(from: q, count: c &- offset) + let q = _childrenStart + slot.value + (q + 1).moveInitialize(from: q, count: c &- slot.value) q.initialize(to: child) } - /// Remove and return the item at `offset`, increasing the amount of free + /// Remove and return the item at `slot`, increasing the amount of free /// space available in the node. /// /// `itemMap` must not yet reflect the removal at the time this /// function is called. This method does not update `itemMap`. @inlinable - internal func _removeItem(at offset: Int) -> Element { + internal func _removeItem(at slot: _Slot) -> Element { assertMutable() let c = itemCount - assert(offset >= 0 && offset < c) + assert(slot.value < c) let stride = MemoryLayout.stride bytesFree &+= stride @@ -76,30 +76,30 @@ extension _Node.UnsafeHandle { .advanced(by: byteCapacity &- stride &* c) .assumingMemoryBound(to: Element.self) - let prefix = c &- 1 &- offset + let prefix = c &- 1 &- slot.value let q = start + prefix let item = q.move() (start + 1).moveInitialize(from: start, count: prefix) return item } - /// Remove and return the child at `offset`, increasing the amount of free + /// Remove and return the child at `slot`, increasing the amount of free /// space available in the node. /// /// `childMap` must not yet reflect the removal at the time this /// function is called. This method does not update `childMap`. @inlinable - internal func _removeChild(at offset: Int) -> _Node { + internal func _removeChild(at slot: _Slot) -> _Node { assertMutable() assert(!isCollisionNode) let count = childCount - assert(offset >= 0 && offset < count) + assert(slot.value < count) bytesFree &+= MemoryLayout<_Node>.stride - let q = _childrenStart + offset + let q = _childrenStart + slot.value let child = q.move() - q.moveInitialize(from: q + 1, count: count &- 1 &- offset) + q.moveInitialize(from: q + 1, count: count &- 1 &- slot.value) return child } } @@ -109,19 +109,19 @@ extension _Node { internal mutating func insertItem( _ item: __owned Element, _ bucket: _Bucket ) { - let offset = read { $0.itemMap.offset(of: bucket) } - self.insertItem(item, at: offset, bucket) + let slot = read { $0.itemMap.slot(of: bucket) } + self.insertItem(item, at: slot, bucket) } - /// Insert `item` at `offset` corresponding to `bucket`. + /// Insert `item` at `slot` corresponding to `bucket`. /// There must be enough free space in the node to fit the new item. @inlinable internal mutating func insertItem( - _ item: __owned Element, at offset: Int, _ bucket: _Bucket + _ item: __owned Element, at slot: _Slot, _ bucket: _Bucket ) { self.count &+= 1 update { - $0._insertItem(item, at: offset) + $0._insertItem(item, at: slot) if $0.isCollisionNode { assert(bucket.isInvalid) $0.collisionCount &+= 1 @@ -129,7 +129,7 @@ extension _Node { assert(!$0.itemMap.contains(bucket)) assert(!$0.childMap.contains(bucket)) $0.itemMap.insert(bucket) - assert($0.itemMap.offset(of: bucket) == offset) + assert($0.itemMap.slot(of: bucket) == slot) } } } @@ -146,30 +146,30 @@ extension _Node { assert(!$0.itemMap.contains(bucket)) assert(!$0.childMap.contains(bucket)) - let offset = $0.childMap.offset(of: bucket) - $0._insertChild(child, at: offset) + let slot = $0.childMap.slot(of: bucket) + $0._insertChild(child, at: slot) $0.childMap.insert(bucket) } } - /// Remove and return the item at `offset`, increasing the amount of free + /// Remove and return the item at `slot`, increasing the amount of free /// space available in the node. @inlinable internal mutating func removeItem( - at offset: Int, _ bucket: _Bucket + at slot: _Slot, _ bucket: _Bucket ) -> Element { defer { _invariantCheck() } assert(count > 0) count &-= 1 return update { - let old = $0._removeItem(at: offset) + let old = $0._removeItem(at: slot) if $0.isCollisionNode { assert(bucket.isInvalid) - assert(offset >= 0 && offset < $0.collisionCount) + assert(slot.value < $0.collisionCount) $0.collisionCount &-= 1 } else { assert($0.itemMap.contains(bucket)) - assert($0.itemMap.offset(of: bucket) == offset) + assert($0.itemMap.slot(of: bucket) == slot) $0.itemMap.remove(bucket) } return old @@ -178,14 +178,14 @@ extension _Node { @inlinable internal mutating func removeChild( - at offset: Int, _ bucket: _Bucket + at slot: _Slot, _ bucket: _Bucket ) -> _Node { defer { _invariantCheck() } assert(!isCollisionNode) let child = update { assert($0.childMap.contains(bucket)) - assert($0.childMap.offset(of: bucket) == offset) - let child = $0._removeChild(at: offset) + assert($0.childMap.slot(of: bucket) == slot) + let child = $0._removeChild(at: slot) $0.childMap.remove(bucket) return child } @@ -201,7 +201,7 @@ extension _Node { count = 0 return update { assert($0.itemCount == 1 && $0.childCount == 0) - let old = $0._removeItem(at: 0) + let old = $0._removeItem(at: .zero) $0.itemMap = .empty $0.childMap = .empty return old @@ -213,7 +213,7 @@ extension _Node { defer { _invariantCheck() } let child = update { assert($0.itemCount == 0 && $0.childCount == 1) - let child = $0._removeChild(at: 0) + let child = $0._removeChild(at: .zero) $0.childMap = .empty return child } @@ -237,29 +237,29 @@ extension _Node { let isUnique = self.isUnique() let r = find(level, key, hash, forInsert: true) switch r { - case .found(_, let offset): + case .found(_, let slot): ensureUnique(isUnique: isUnique) _invariantCheck() // FIXME return update { - let p = $0.itemPtr(at: offset) + let p = $0.itemPtr(at: slot) let old = p.pointee.value p.pointee.value = value return old } - case .notFound(let bucket, let offset): + case .notFound(let bucket, let slot): ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) - insertItem((key, value), at: offset, bucket) + insertItem((key, value), at: slot, bucket) return nil - case .newCollision(let bucket, let offset): - let existingHash = read { _Hash($0[item: offset].key) } + case .newCollision(let bucket, let slot): + let existingHash = read { _Hash($0[item: slot].key) } if hash == existingHash, hasSingletonItem { // Convert current node to a collision node. ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) update { $0.collisionCount = 1 } - insertItem((key, value), at: 1, .invalid) + insertItem((key, value), at: _Slot(1), .invalid) } else { ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) - let existing = removeItem(at: offset, bucket) + let existing = removeItem(at: slot, bucket) let node = _Node( level: level.descend(), item1: existing, existingHash, @@ -273,10 +273,10 @@ extension _Node { item1: (key, value), hash, child2: self, collisionHash) return nil - case .descend(_, let offset): + case .descend(_, let slot): ensureUnique(isUnique: isUnique) let old = update { - $0[child: offset].updateValue(value, forKey: key, level.descend(), hash) + $0[child: slot].updateValue(value, forKey: key, level.descend(), hash) } if old == nil { count &+= 1 } return old @@ -302,8 +302,8 @@ extension _Node { } let r = find(level, key, hash, forInsert: false) switch r { - case .found(let bucket, let offset): - let old = removeItem(at: offset, bucket) + case .found(let bucket, let slot): + let old = removeItem(at: slot, bucket) if isAtrophied { self = removeSingletonChild() } @@ -313,9 +313,9 @@ extension _Node { return old case .notFound, .newCollision, .expansion: return nil - case .descend(let bucket, let offset): + case .descend(let bucket, let slot): let (old, needsInlining) = update { - let child = $0.childPtr(at: offset) + let child = $0.childPtr(at: slot) let old = child.pointee.remove(key, level.descend(), hash) guard old != nil else { return (old, false) } let needsInlining = child.pointee.hasSingletonItem @@ -325,7 +325,7 @@ extension _Node { count &-= 1 if needsInlining { ensureUnique(isUnique: true, withFreeSpace: Self.spaceForInlinedChild) - var child = self.removeChild(at: offset, bucket) + var child = self.removeChild(at: slot, bucket) let item = child.removeSingletonItem() insertItem(item, bucket) } @@ -341,7 +341,7 @@ extension _Node { assert(isCollisionNode && hasSingletonItem) assert(isUnique()) update { - let hash = _Hash($0[item: 0].key) + let hash = _Hash($0[item: .zero].key) $0.itemMap = _Bitmap(hash[.top]) $0.childMap = .empty } @@ -353,16 +353,18 @@ extension _Node { ) -> (replacement: _Node, old: Element)? { let r = find(level, key, hash, forInsert: false) switch r { - case .found(let bucket, let offset): + case .found(let bucket, let slot): // Don't copy the node if we'd immediately discard it. let willAtrophy = read { - $0.itemCount == 1 && $0.childCount == 1 && $0[child: 0].isCollisionNode + $0.itemCount == 1 + && $0.childCount == 1 + && $0[child: .zero].isCollisionNode } if willAtrophy { - return read { ($0[child: 0], $0[item: 0]) } + return read { ($0[child: .zero], $0[item: .zero]) } } var node = self.copy() - let old = node.removeItem(at: offset, bucket) + let old = node.removeItem(at: slot, bucket) if level.isAtRoot, node.isCollisionNode, node.hasSingletonItem { node._convertToRegularNode() } @@ -370,12 +372,12 @@ extension _Node { return (node, old) case .notFound, .newCollision, .expansion: return nil - case .descend(let bucket, let offset): - let r = read { $0[child: offset].removing(key, level.descend(), hash) } + case .descend(let bucket, let slot): + let r = read { $0[child: slot].removing(key, level.descend(), hash) } guard var r = r else { return nil } if r.replacement.hasSingletonItem { var node = copy(withFreeSpace: Self.spaceForInlinedChild) - _ = node.removeChild(at: offset, bucket) + _ = node.removeChild(at: slot, bucket) let item = r.replacement.removeSingletonItem() node.insertItem(item, bucket) node._invariantCheck() @@ -386,7 +388,7 @@ extension _Node { return (r.replacement, r.old) } var node = copy() - node.update { $0[child: offset] = r.replacement } + node.update { $0[child: slot] = r.replacement } node.count &-= 1 node._invariantCheck() return (node, r.old) diff --git a/Sources/PersistentCollections/Node/_Node+Storage.swift b/Sources/PersistentCollections/Node/_Node+Storage.swift index 0b510f1dc..413ab5021 100644 --- a/Sources/PersistentCollections/Node/_Node+Storage.swift +++ b/Sources/PersistentCollections/Node/_Node+Storage.swift @@ -195,7 +195,9 @@ extension _Node { assert(target.bytesFree >= space) } } + self.count = 0 self._invariantCheck() + new._invariantCheck() self = new } } diff --git a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index e11dfcda4..e5c9b685e 100644 --- a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -149,21 +149,26 @@ extension _Node.UnsafeHandle { _header.pointee.childCount } + @inlinable @inline(__always) + internal var childEnd: _Slot { + _header.pointee.childEnd + } + @inlinable internal var _children: UnsafeMutableBufferPointer<_Node> { UnsafeMutableBufferPointer(start: _childrenStart, count: childCount) } @inlinable - internal func childPtr(at offset: Int) -> UnsafeMutablePointer<_Node> { - assert(offset >= 0 && offset < childCount) - return _childrenStart + offset + internal func childPtr(at slot: _Slot) -> UnsafeMutablePointer<_Node> { + assert(slot.value < childCount) + return _childrenStart + slot.value } @inlinable - internal subscript(child offset: Int) -> _Node { - unsafeAddress { UnsafePointer(childPtr(at: offset)) } - nonmutating unsafeMutableAddress { childPtr(at: offset) } + internal subscript(child slot: _Slot) -> _Node { + unsafeAddress { UnsafePointer(childPtr(at: slot)) } + nonmutating unsafeMutableAddress { childPtr(at: slot) } } @inlinable @@ -182,6 +187,11 @@ extension _Node.UnsafeHandle { _header.pointee.itemCount } + @inlinable @inline(__always) + internal var itemEnd: _Slot { + _header.pointee.itemEnd + } + @inlinable internal var reverseItems: UnsafeMutableBufferPointer { let c = itemCount @@ -189,15 +199,15 @@ extension _Node.UnsafeHandle { } @inlinable - internal func itemPtr(at offset: Int) -> UnsafeMutablePointer { - assert(offset >= 0 && offset < itemCount) - return _itemsEnd.advanced(by: -1 &- offset) + internal func itemPtr(at slot: _Slot) -> UnsafeMutablePointer { + assert(slot.value < itemCount) + return _itemsEnd.advanced(by: -1 &- slot.value) } @inlinable - internal subscript(item offset: Int) -> Element { - unsafeAddress { UnsafePointer(itemPtr(at: offset)) } - unsafeMutableAddress { itemPtr(at: offset) } + internal subscript(item slot: _Slot) -> Element { + unsafeAddress { UnsafePointer(itemPtr(at: slot)) } + unsafeMutableAddress { itemPtr(at: slot) } } } @@ -214,6 +224,6 @@ extension _Node.UnsafeHandle { @inlinable internal var isAtrophiedNode: Bool { - hasSingletonChild && self[child: 0].isCollisionNode + hasSingletonChild && self[child: .zero].isCollisionNode } } diff --git a/Sources/PersistentCollections/Node/_Node.swift b/Sources/PersistentCollections/Node/_Node.swift index c57f682ee..876c22ab7 100644 --- a/Sources/PersistentCollections/Node/_Node.swift +++ b/Sources/PersistentCollections/Node/_Node.swift @@ -26,7 +26,8 @@ /// └───┴───┴───┴───┴───┴──────────────────┴───┴───┴───┴───┘ /// children items /// -/// Note that the region for items grows downwards from the end. +/// Note that the region for items grows downwards from the end, so the item +/// at slot 0 is at the very end of the buffer. /// /// Two 32-bit bitmaps are used to associate each child and item with their /// position in the hash table. The bucket occupied by the *n*th child in the diff --git a/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift index 6017e0da2..a02556a8c 100644 --- a/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift @@ -46,6 +46,11 @@ extension _RawNode.UnsafeHandle { _header.pointee.childCount } + @inline(__always) + internal var childEnd: _Slot { + _header.pointee.childEnd + } + @inline(__always) internal var hasItems: Bool { _header.pointee.hasItems @@ -56,16 +61,20 @@ extension _RawNode.UnsafeHandle { _header.pointee.itemCount } + @inline(__always) + internal var itemEnd: _Slot { + _header.pointee.itemEnd + } @inline(__always) internal var _childrenStart: UnsafePointer<_RawNode> { _memory.assumingMemoryBound(to: _RawNode.self) } - internal subscript(child offset: Int) -> _RawNode { + internal subscript(child slot: _Slot) -> _RawNode { unsafeAddress { - assert(offset >= 0 && offset < childCount) - return _childrenStart + offset + assert(slot < childEnd) + return _childrenStart + slot.value } } diff --git a/Sources/PersistentCollections/Node/_RawNode.swift b/Sources/PersistentCollections/Node/_RawNode.swift index e8ab9c910..41bdeba24 100644 --- a/Sources/PersistentCollections/Node/_RawNode.swift +++ b/Sources/PersistentCollections/Node/_RawNode.swift @@ -52,16 +52,16 @@ extension _RawNode { var l = _Level.top var n = self.unmanaged while l < path.level { - let offset = path.ancestors[l] - precondition(offset < n.childCount) - n = n.unmanagedChild(at: offset) + let slot = path.ancestors[l] + precondition(slot < n.childEnd) + n = n.unmanagedChild(at: slot) l = l.descend() } precondition(n == path.node) if path._isItem { - precondition(path._nodeOffset < n.itemCount) + precondition(path.nodeSlot < n.itemEnd) } else { - precondition(path._nodeOffset <= n.childCount) + precondition(path.nodeSlot <= n.childEnd) } } } diff --git a/Sources/PersistentCollections/Node/_Slot.swift b/Sources/PersistentCollections/Node/_Slot.swift new file mode 100644 index 000000000..7fe74fcd6 --- /dev/null +++ b/Sources/PersistentCollections/Node/_Slot.swift @@ -0,0 +1,104 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift Collections open source project +// +// Copyright (c) 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// +//===----------------------------------------------------------------------===// + + +@usableFromInline +@frozen +internal struct _Slot { + @usableFromInline + internal var _value: UInt32 + + @inlinable @inline(__always) + internal init(_ value: UInt32) { + self._value = value + } + + @inlinable @inline(__always) + internal init(_ value: UInt) { + assert(value <= UInt32.max) + self._value = UInt32(truncatingIfNeeded: value) + } + + @inlinable @inline(__always) + internal init(_ value: Int) { + assert(value >= 0 && value <= UInt32.max) + self._value = UInt32(truncatingIfNeeded: value) + } +} + +extension _Slot { + @inlinable @inline(__always) + internal static var zero: _Slot { _Slot(0) } +} + +extension _Slot { + @inlinable @inline(__always) + internal var value: Int { + Int(truncatingIfNeeded: _value) + } +} + +extension _Slot: Equatable { + @inlinable @inline(__always) + internal static func ==(left: Self, right: Self) -> Bool { + left._value == right._value + } +} + +extension _Slot: Comparable { + @inlinable @inline(__always) + internal static func <(left: Self, right: Self) -> Bool { + left._value < right._value + } +} + +extension _Slot: Hashable { + @inlinable + internal func hash(into hasher: inout Hasher) { + hasher.combine(_value) + } +} + +extension _Slot: CustomStringConvertible { + @usableFromInline + internal var description: String { + "\(_value)" + } +} + +extension _Slot: Strideable { + @inlinable @inline(__always) + internal func advanced(by n: Int) -> _Slot { + return _Slot(_value &+ UInt32(truncatingIfNeeded: n)) + } + + @inlinable @inline(__always) + internal func distance(to other: _Slot) -> Int { + if self < other { + return Int(truncatingIfNeeded: other._value - self._value) + } + return -Int(truncatingIfNeeded: self._value - other._value) + } +} + +extension _Slot { + @inlinable @inline(__always) + internal func next() -> _Slot { + assert(_value < .max) + return _Slot(_value &+ 1) + } + + @inlinable @inline(__always) + internal func previous() -> _Slot { + assert(_value > 0) + return _Slot(_value &- 1) + } +} diff --git a/Sources/PersistentCollections/Node/_StorageHeader.swift b/Sources/PersistentCollections/Node/_StorageHeader.swift index d0f568a55..df8a4516e 100644 --- a/Sources/PersistentCollections/Node/_StorageHeader.swift +++ b/Sources/PersistentCollections/Node/_StorageHeader.swift @@ -84,6 +84,16 @@ extension _StorageHeader { : itemMap.count) } + @inlinable @inline(__always) + internal var childEnd: _Slot { + _Slot(childCount) + } + + @inlinable @inline(__always) + internal var itemEnd: _Slot { + _Slot(itemCount) + } + @inlinable internal var collisionCount: Int { get { diff --git a/Sources/PersistentCollections/Node/_UnmanagedNode.swift b/Sources/PersistentCollections/Node/_UnmanagedNode.swift index 03ae07daf..d7d7e1ea8 100644 --- a/Sources/PersistentCollections/Node/_UnmanagedNode.swift +++ b/Sources/PersistentCollections/Node/_UnmanagedNode.swift @@ -73,11 +73,21 @@ extension _UnmanagedNode { } @inlinable - internal func unmanagedChild(at offset: Int) -> Self { + internal var itemEnd: _Slot { + withRaw { _Slot($0.header.itemCount) } + } + + @inlinable + internal var childEnd: _Slot { + withRaw { _Slot($0.header.childCount) } + } + + @inlinable + internal func unmanagedChild(at slot: _Slot) -> Self { withRaw { raw in - assert(offset >= 0 && offset < raw.header.childCount) + assert(slot.value < raw.header.childCount) return raw.withUnsafeMutablePointerToElements { p in - Self(p[offset].storage) + Self(p[slot.value].storage) } } } diff --git a/Sources/PersistentCollections/Node/_UnsafePath.swift b/Sources/PersistentCollections/Node/_UnsafePath.swift index cbd2e9c21..4c21c6171 100644 --- a/Sources/PersistentCollections/Node/_UnsafePath.swift +++ b/Sources/PersistentCollections/Node/_UnsafePath.swift @@ -12,13 +12,13 @@ @usableFromInline internal struct _UnsafePath { @usableFromInline - internal var ancestors: _AncestorOffsets + internal var ancestors: _AncestorSlots @usableFromInline internal var node: _UnmanagedNode @usableFromInline - internal var _nodeOffset: UInt32 + internal var nodeSlot: _Slot @usableFromInline internal var level: _Level @@ -32,7 +32,7 @@ internal struct _UnsafePath { self.level = .top self.ancestors = .empty self.node = root.unmanaged - self._nodeOffset = 0 + self.nodeSlot = .zero self._isItem = root.storage.header.hasItems } } @@ -40,15 +40,15 @@ internal struct _UnsafePath { extension _UnsafePath { internal init( _ level: _Level, - _ ancestors: _AncestorOffsets, + _ ancestors: _AncestorSlots, _ node: _UnmanagedNode, - _ childOffset: Int + _ childSlot: _Slot ) { - assert(childOffset >= 0 && childOffset < node.childCount) + assert(childSlot < node.childEnd) self.level = level self.ancestors = ancestors self.node = node - self._nodeOffset = UInt32(truncatingIfNeeded: childOffset) + self.nodeSlot = childSlot self._isItem = false } } @@ -60,7 +60,7 @@ extension _UnsafePath: Equatable { // Note: we don't compare nodes (node equality should follow from the rest) left.level == right.level && left.ancestors == right.ancestors - && left._nodeOffset == right._nodeOffset + && left.nodeSlot == right.nodeSlot && left._isItem == right._isItem } } @@ -71,7 +71,7 @@ extension _UnsafePath: Hashable { internal func hash(into hasher: inout Hasher) { // Note: we don't hash nodes, as they aren't compared by ==, either. hasher.combine(ancestors.path) - hasher.combine(_nodeOffset) + hasher.combine(nodeSlot) hasher.combine(level._shift) hasher.combine(_isItem) } @@ -81,7 +81,7 @@ extension _UnsafePath: Comparable { @usableFromInline @_effects(releasenone) internal static func <(left: Self, right: Self) -> Bool { - // This implements a total ordering across paths based on the offset + // This implements a total ordering across paths based on the slot // sequences they contain, corresponding to a preorder walk of the tree. // // Paths addressing items within a node are ordered before paths addressing @@ -98,18 +98,18 @@ extension _UnsafePath: Comparable { assert(level < right.level || !right.ancestors.hasDataBelow(level)) if level < right.level { guard !left._isItem else { return true } - let l = left._nodeOffset + let l = left.nodeSlot let r = right.ancestors[level] return l < r } if level < left.level { guard !right._isItem else { return false } let l = left.ancestors[level] - let r = right._nodeOffset + let r = right.nodeSlot return l < r } guard left._isItem == right._isItem else { return left._isItem } - return left._nodeOffset < right._nodeOffset + return left.nodeSlot < right.nodeSlot } } @@ -123,13 +123,13 @@ extension _UnsafePath: CustomStringConvertible { l = l.descend() } if isPlaceholder { - d += "[\(self._nodeOffset)]?" + d += "[\(self.nodeSlot)]?" } else if isOnItem { - d += "[\(self._nodeOffset)]" + d += "[\(self.nodeSlot)]" } else if isOnChild { - d += ".\(self._nodeOffset)" + d += ".\(self.nodeSlot)" } else if isOnNodeEnd { - d += ".$(\(self._nodeOffset))" + d += ".$(\(self.nodeSlot))" } return d } @@ -145,7 +145,7 @@ extension _UnsafePath { /// trigger undefined behavior. @inlinable @inline(__always) internal var isOnItem: Bool { - // Note: this may be true even if nodeOffset == itemCount (insertion paths). + // Note: this may be true even if nodeSlot == itemCount (insertion paths). _isItem } @@ -154,7 +154,7 @@ extension _UnsafePath { /// inserted later; they do not occur while simply iterating over existing /// items. internal var isPlaceholder: Bool { - _isItem && _nodeOffset == node.itemCount + _isItem && nodeSlot.value == node.itemCount } /// Returns true if this path addresses a node in the tree; otherwise returns @@ -165,7 +165,7 @@ extension _UnsafePath { /// never get called when the node is no longer valid; otherwise this will /// trigger undefined behavior. internal var isOnChild: Bool { - !_isItem && _nodeOffset < node.childCount + !_isItem && nodeSlot.value < node.childCount } /// Returns true if this path addresses an empty slot within a node in a tree; @@ -176,29 +176,18 @@ extension _UnsafePath { /// never get called when the node is no longer valid; otherwise this will /// trigger undefined behavior. internal var isOnNodeEnd: Bool { - !_isItem && _nodeOffset == node.childCount + !_isItem && nodeSlot.value == node.childCount } @inlinable internal var isOnLeftmostItem: Bool { // We are on the leftmost item in the tree if we are currently // addressing an item and the ancestors path is all zero bits. - _isItem && ancestors == .empty && _nodeOffset == 0 + _isItem && ancestors == .empty && nodeSlot == .zero } } extension _UnsafePath { - @inlinable @inline(__always) - internal var nodeOffset: Int { - get { - Int(truncatingIfNeeded: _nodeOffset) - } - set { - assert(newValue >= 0 && newValue <= UInt32.max) - _nodeOffset = UInt32(truncatingIfNeeded: newValue) - } - } - /// Returns an unmanaged reference to the child node this path is currently /// addressing. /// @@ -208,28 +197,28 @@ extension _UnsafePath { /// trigger undefined behavior. internal var currentChild: _UnmanagedNode { assert(isOnChild) - return node.unmanagedChild(at: nodeOffset) + return node.unmanagedChild(at: nodeSlot) } - internal func childOffset(at level: _Level) -> Int { + internal func childSlot(at level: _Level) -> _Slot { assert(level < self.level) return ancestors[level] } - /// Returns the offset to the currently addressed item. + /// Returns the slot of the currently addressed item. /// /// - Note: This method needs to resolve the unmanaged node reference /// that is stored in the path. It is up to the caller to ensure this will /// never get called when the node is no longer valid; otherwise this will /// trigger undefined behavior. @inlinable @inline(__always) - internal var currentItemOffset: Int { + internal var currentItemSlot: _Slot { assert(isOnItem) - return nodeOffset + return nodeSlot } } extension _UnsafePath { - /// Positions this path on the item with the specified offset within its + /// Positions this path on the item with the specified slot within its /// current node. /// /// - Note: This method needs to resolve the unmanaged node reference @@ -237,16 +226,16 @@ extension _UnsafePath { /// never get called when the node is no longer valid; otherwise this will /// trigger undefined behavior. @inlinable - internal mutating func selectItem(at offset: Int) { - // As a special exception, this allows offset to equal the item count. + internal mutating func selectItem(at slot: _Slot) { + // As a special exception, this allows slot to equal the item count. // This can happen for paths that address the position a new item might be // inserted later. - assert(offset >= 0 && offset <= node.itemCount) - nodeOffset = offset + assert(slot <= node.itemEnd) + nodeSlot = slot _isItem = true } - /// Positions this path on the child with the specified offset within its + /// Positions this path on the child with the specified slot within its /// current node, without descending into it. /// /// - Note: This method needs to resolve the unmanaged node reference @@ -254,11 +243,11 @@ extension _UnsafePath { /// never get called when the node is no longer valid; otherwise this will /// trigger undefined behavior. @inlinable - internal mutating func selectChild(at offset: Int) { - // As a special exception, this allows offset to equal the child count. + internal mutating func selectChild(at slot: _Slot) { + // As a special exception, this allows slot to equal the child count. // This is equivalent to a call to `selectEnd()`. - assert(offset >= 0 && offset <= node.childCount) - nodeOffset = offset + assert(slot <= node.childEnd) + nodeSlot = slot _isItem = false } @@ -271,7 +260,7 @@ extension _UnsafePath { @usableFromInline @_effects(releasenone) internal mutating func selectEnd() { - nodeOffset = node.childCount + nodeSlot = node.childEnd _isItem = false } @@ -288,26 +277,26 @@ extension _UnsafePath { @_effects(releasenone) internal mutating func descend() { self.node = currentChild - self.ancestors[level] = nodeOffset - self.nodeOffset = 0 + self.ancestors[level] = nodeSlot + self.nodeSlot = .zero self._isItem = node.hasItems self.level = level.descend() } internal mutating func ascendToNearestAncestor( under root: _RawNode, - where test: (_UnmanagedNode, Int) -> Bool + where test: (_UnmanagedNode, _Slot) -> Bool ) -> Bool { if self.level.isAtRoot { return false } var best: _UnsafePath? = nil var n = root.unmanaged var l: _Level = .top while l < self.level { - let offset = self.ancestors[l] - if test(n, offset) { - best = _UnsafePath(l, self.ancestors.truncating(to: l), n, offset) + let slot = self.ancestors[l] + if test(n, slot) { + best = _UnsafePath(l, self.ancestors.truncating(to: l), n, slot) } - n = n.unmanagedChild(at: offset) + n = n.unmanagedChild(at: slot) l = l.descend() } guard let best = best else { return false } @@ -325,7 +314,7 @@ extension _UnsafePath { } assert(l.descend() == self.level) self._isItem = false - self.nodeOffset = self.ancestors[l] + self.nodeSlot = self.ancestors[l] let oldNode = self.node self.node = n self.ancestors.clear(l) @@ -337,19 +326,19 @@ extension _UnsafePath { extension _UnsafePath { mutating func selectNextItem() -> Bool { assert(isOnItem) - _nodeOffset &+= 1 - if _nodeOffset < node.itemCount { return true } - _nodeOffset = 0 + nodeSlot = nodeSlot.next() + if nodeSlot < node.itemEnd { return true } + nodeSlot = .zero _isItem = false return false } mutating func selectNextChild() -> Bool { assert(!isOnItem) - let childCount = node.childCount - guard _nodeOffset < childCount else { return false } - _nodeOffset &+= 1 - return _nodeOffset < childCount + let childEnd = node.childEnd + guard nodeSlot < childEnd else { return false } + nodeSlot = nodeSlot.next() + return nodeSlot < childEnd } } @@ -366,13 +355,13 @@ extension _UnsafePath { assert(isOnChild) while true { descend() - let childCount = node.childCount - guard childCount > 0 else { break } - selectChild(at: childCount - 1) + let childEnd = node.childEnd + guard childEnd > .zero else { break } + selectChild(at: childEnd.previous()) } - let itemCount = node.itemCount - assert(itemCount > 0) - selectItem(at: itemCount - 1) + let itemEnd = node.itemEnd + assert(itemEnd > .zero) + selectItem(at: itemEnd.previous()) } @usableFromInline @@ -386,7 +375,7 @@ extension _UnsafePath { return true } if ascendToNearestAncestor( - under: root, where: { $1 &+ 1 < $0.childCount } + under: root, where: { $1.next() < $0.childEnd } ) { let r = selectNextChild() assert(r) @@ -402,33 +391,35 @@ extension _UnsafePath { @usableFromInline @_effects(releasenone) internal mutating func findPredecessorItem(under root: _RawNode) -> Bool { - switch (isOnItem, nodeOffset > 0) { + switch (isOnItem, nodeSlot > .zero) { case (true, true): - selectItem(at: nodeOffset &- 1) + selectItem(at: nodeSlot.previous()) return true case (false, true): - selectChild(at: nodeOffset &- 1) + selectChild(at: nodeSlot.previous()) descendToRightMostItem() return true case (false, false): if node.hasItems { - selectItem(at: node.itemCount &- 1) + selectItem(at: node.itemEnd.previous()) return true } case (true, false): break } guard - ascendToNearestAncestor(under: root, where: { $0.hasItems || $1 > 0 }) + ascendToNearestAncestor( + under: root, + where: { $0.hasItems || $1 > .zero }) else { return false } - if nodeOffset > 0 { - selectChild(at: nodeOffset &- 1) + if nodeSlot > .zero { + selectChild(at: nodeSlot.previous()) descendToRightMostItem() return true } if node.hasItems { - selectItem(at: node.itemCount &- 1) + selectItem(at: node.itemEnd.previous()) return true } return false @@ -436,21 +427,23 @@ extension _UnsafePath { } extension _RawNode { - internal func preorderPosition(_ level: _Level, of path: _UnsafePath) -> Int { + internal func preorderPosition( + _ level: _Level, of path: _UnsafePath + ) -> Int { if path.isOnNodeEnd { return count } assert(path.isOnItem) if level < path.level { - let childOffset = path.childOffset(at: level) + let childSlot = path.childSlot(at: level) return read { - let prefix = $0._children[.. Int { read { - assert(start >= 0 && start < $0.itemCount) + assert(start < $0.itemEnd) assert(level < end.level) - let childOffset = end.childOffset(at: level) + let childSlot = end.childSlot(at: level) let children = $0._children - let prefix = children[.. Date: Fri, 16 Sep 2022 01:31:15 -0700 Subject: [PATCH 10/18] [test] LifetimeTracked: Implement high-fidelity hash forwarding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RawCollider` doesn’t like to get called through the high-level hashing APIs. --- .../Utilities/LifetimeTracked.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/_CollectionsTestSupport/Utilities/LifetimeTracked.swift b/Sources/_CollectionsTestSupport/Utilities/LifetimeTracked.swift index a53b57951..207843a9a 100644 --- a/Sources/_CollectionsTestSupport/Utilities/LifetimeTracked.swift +++ b/Sources/_CollectionsTestSupport/Utilities/LifetimeTracked.swift @@ -59,9 +59,17 @@ extension LifetimeTracked: Equatable where Payload: Equatable { } extension LifetimeTracked: Hashable where Payload: Hashable { + public func _rawHashValue(seed: Int) -> Int { + payload._rawHashValue(seed: seed) + } + public func hash(into hasher: inout Hasher) { hasher.combine(payload) } + + public var hashValue: Int { + payload.hashValue + } } extension LifetimeTracked: Comparable where Payload: Comparable { From 6cf128e6eacdf5bf419debe16e6d856215aee0a0 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Fri, 16 Sep 2022 14:49:37 -0700 Subject: [PATCH 11/18] [PersistentCollections] Internal doc updates --- .../Node/_AncestorOffsets.swift | 30 +++- .../PersistentCollections/Node/_Level.swift | 10 ++ .../Node/_Node+UnsafeHandle.swift | 10 ++ .../Node/_RawNode+UnsafeHandle.swift | 12 ++ .../PersistentCollections/Node/_RawNode.swift | 6 + .../PersistentCollections/Node/_Slot.swift | 8 +- .../Node/_StorageHeader.swift | 3 + .../Node/_UnmanagedNode.swift | 12 ++ .../Node/_UnsafePath.swift | 149 +++++++++++++----- 9 files changed, 195 insertions(+), 45 deletions(-) diff --git a/Sources/PersistentCollections/Node/_AncestorOffsets.swift b/Sources/PersistentCollections/Node/_AncestorOffsets.swift index 297fbb772..2a6f793a1 100644 --- a/Sources/PersistentCollections/Node/_AncestorOffsets.swift +++ b/Sources/PersistentCollections/Node/_AncestorOffsets.swift @@ -9,6 +9,14 @@ // //===----------------------------------------------------------------------===// +/// A collection of slot values logically addressing a particular node in a +/// hash tree. The collection is (logically) extended with zero slots up to +/// the maximum depth of the tree -- to unambiguously address a single node, +/// this therefore needs to be augmented with a `_Level` value. +/// +/// This construct can only be used to identify a particular node in the tree; +/// it does not necessarily have room to include an item offset in the addressed +/// node. (See `_Path` if you need to address a particular item.) @usableFromInline @frozen struct _AncestorSlots { @@ -34,6 +42,9 @@ extension _AncestorSlots { } extension _AncestorSlots { + /// Return or set the slot value at the specified level. + /// If this is used to mutate the collection, then the original value + /// on the given level must be zero. @inlinable @inline(__always) internal subscript(_ level: _Level) -> _Slot { get { @@ -47,6 +58,15 @@ extension _AncestorSlots { } } + /// Clear the slot at the specified level, by setting it to zero. + @inlinable + internal mutating func clear(_ level: _Level) { + guard level.shift < UInt.bitWidth else { return } + path &= ~(_Bucket.bitMask &<< level.shift) + } + + /// Truncate this path to the specified level. + /// Slots at or beyond the specified level are cleared. @inlinable internal func truncating(to level: _Level) -> _AncestorSlots { assert(level.shift <= UInt.bitWidth) @@ -54,18 +74,16 @@ extension _AncestorSlots { return _AncestorSlots(path & ((1 &<< level.shift) &- 1)) } - @inlinable - internal mutating func clear(_ level: _Level) { - guard level.shift < UInt.bitWidth else { return } - path &= ~(_Bucket.bitMask &<< level.shift) - } - + /// Returns true if this path contains non-zero slots at or beyond the + /// specified level, otherwise returns false. @inlinable internal func hasDataBelow(_ level: _Level) -> Bool { guard level.shift < UInt.bitWidth else { return false } return (path &>> level.shift) != 0 } + /// Compares this path to `other` up to but not including the specified level. + /// Returns true if the path prefixes compare equal, otherwise returns false. @inlinable internal func isEqual(to other: Self, upTo level: _Level) -> Bool { if level.isAtRoot { return true } diff --git a/Sources/PersistentCollections/Node/_Level.swift b/Sources/PersistentCollections/Node/_Level.swift index 04c33bbd7..a4f3f7d4a 100644 --- a/Sources/PersistentCollections/Node/_Level.swift +++ b/Sources/PersistentCollections/Node/_Level.swift @@ -9,9 +9,16 @@ // //===----------------------------------------------------------------------===// +/// Identifies a particular level within the hash tree. +/// +/// Hash trees have a maximum depth of ⎡`UInt.bitWidth / _Bucket.bitWidth`⎤, so +/// the level always fits in an `UInt8` value. @usableFromInline @frozen internal struct _Level { + /// The bit position within a hash value that begins the hash slice that is + /// associated with this level. For collision nodes, this can be larger than + /// `UInt.bitWidth`. @usableFromInline internal var _shift: UInt8 @@ -43,6 +50,9 @@ extension _Level { _Level(shift: 0) } + /// The bit position within a hash value that begins the hash slice that is + /// associated with this level. For collision nodes, this can be larger than + /// `UInt.bitWidth`. @inlinable @inline(__always) internal var shift: UInt { UInt(truncatingIfNeeded: _shift) } diff --git a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index e5c9b685e..78a45c347 100644 --- a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -10,6 +10,16 @@ //===----------------------------------------------------------------------===// extension _Node { + /// An unsafe view of the data stored inside a node in the hash tree, hiding + /// the mechanics of accessing storage from the code that uses it. + /// + /// Handles do not own the storage they access -- it is the client's + /// responsibility to ensure that handles (and any pointer values generated + /// by them) do not escape the closure call that received them. + /// + /// A handle can be either read-only or mutable, depending on the method used + /// to access it. In debug builds, methods that modify data trap at runtime if + /// they're called on a read-only view. @usableFromInline @frozen internal struct UnsafeHandle { diff --git a/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift index a02556a8c..c6b59b987 100644 --- a/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_RawNode+UnsafeHandle.swift @@ -10,6 +10,18 @@ //===----------------------------------------------------------------------===// extension _RawNode { + /// An unsafe, non-generic view of the data stored inside a node in the + /// hash tree, hiding the mechanics of accessing storage from the code that + /// uses it. + /// + /// This is the non-generic equivalent of `_Node.UnsafeHandle`, sharing some + /// of its functionality, but it only provides read-only access to the tree + /// structure (incl. subtree counts) -- it doesn't provide any ways to mutate + /// the underlying data or to access user payload. + /// + /// Handles do not own the storage they access -- it is the client's + /// responsibility to ensure that handles (and any pointer values generated + /// by them) do not escape the closure call that received them. @usableFromInline @frozen internal struct UnsafeHandle { diff --git a/Sources/PersistentCollections/Node/_RawNode.swift b/Sources/PersistentCollections/Node/_RawNode.swift index 41bdeba24..d0f681272 100644 --- a/Sources/PersistentCollections/Node/_RawNode.swift +++ b/Sources/PersistentCollections/Node/_RawNode.swift @@ -9,6 +9,12 @@ // //===----------------------------------------------------------------------===// +/// A type-erased node in a hash tree. This doesn't know about the user data +/// stored in the tree, but it has access to subtree counts and it can be used +/// to freely navigate within the tree structure. +/// +/// This construct is powerful enough to implement APIs such as `index(after:)`, +/// `distance(from:to:)`, `index(_:offsetBy:)` in non-generic code. @usableFromInline @frozen internal struct _RawNode { diff --git a/Sources/PersistentCollections/Node/_Slot.swift b/Sources/PersistentCollections/Node/_Slot.swift index 7fe74fcd6..b22035a26 100644 --- a/Sources/PersistentCollections/Node/_Slot.swift +++ b/Sources/PersistentCollections/Node/_Slot.swift @@ -9,7 +9,13 @@ // //===----------------------------------------------------------------------===// - +/// Identifies a position within a contiguous storage region within a hash tree +/// node. Hash tree nodes have two storage regions, one for items and one for +/// children; the same `_Slot` type is used to refer to positions within both. +/// +/// We use the term "slot" to refer to internal storage entries, to avoid +/// confusion with terms that sometimes appear in public API, such as +/// "index", "position" or "offset". @usableFromInline @frozen internal struct _Slot { diff --git a/Sources/PersistentCollections/Node/_StorageHeader.swift b/Sources/PersistentCollections/Node/_StorageHeader.swift index df8a4516e..21749d71f 100644 --- a/Sources/PersistentCollections/Node/_StorageHeader.swift +++ b/Sources/PersistentCollections/Node/_StorageHeader.swift @@ -9,6 +9,9 @@ // //===----------------------------------------------------------------------===// +/// The storage header in a hash tree node. This includes data about the +/// current size and capacity of the node's storage region, as well as +/// information about the currently occupied hash table buckets. @usableFromInline @frozen internal struct _StorageHeader { diff --git a/Sources/PersistentCollections/Node/_UnmanagedNode.swift b/Sources/PersistentCollections/Node/_UnmanagedNode.swift index d7d7e1ea8..279c5a4de 100644 --- a/Sources/PersistentCollections/Node/_UnmanagedNode.swift +++ b/Sources/PersistentCollections/Node/_UnmanagedNode.swift @@ -11,6 +11,12 @@ import _CollectionsUtilities +/// An unsafe, unowned, type-erased reference to a hash tree node; essentially +/// just a lightweight wrapper around `Unmanaged<_RawStorage>`. +/// +/// Because such a reference may outlive the underlying object, use sites must +/// be extraordinarily careful to never dereference an invalid `_UnmanagedNode`. +/// Doing so results in undefined behavior. @usableFromInline @frozen internal struct _UnmanagedNode { @@ -24,6 +30,12 @@ internal struct _UnmanagedNode { } extension _UnmanagedNode: Equatable { + /// Indicates whether two unmanaged node references are equal. + /// + /// This function is safe to call even if one or both of its arguments are + /// invalid -- however, it may incorrectly return true in this case. + /// (This can happen when a destroyed node's memory region is later reused for + /// a newly created node.) @inlinable internal static func ==(left: Self, right: Self) -> Bool { left.ref.toOpaque() == right.ref.toOpaque() diff --git a/Sources/PersistentCollections/Node/_UnsafePath.swift b/Sources/PersistentCollections/Node/_UnsafePath.swift index 4c21c6171..fd5d39c5c 100644 --- a/Sources/PersistentCollections/Node/_UnsafePath.swift +++ b/Sources/PersistentCollections/Node/_UnsafePath.swift @@ -9,6 +9,30 @@ // //===----------------------------------------------------------------------===// +/// A non-owning, mutable construct representing a path to an item or child node +/// within a hash tree (or the virtual slot addressing the end of the +/// items or children region within a node). +/// +/// Path values provide mutating methods to freely navigate around in the tree, +/// including basics such as descending into a child node, ascending to a +/// parent or selecting a particular item within the current node; as well as +/// more complicated methods such as finding the next/previous item in a +/// preorder walk of the tree. +/// +/// Paths are, for the most part, represented by a series of slot values +/// identifying a particular branch within each level in the tree up to and +/// including the final node on the path. +/// +/// However, to speed up common operations, path values also include a single +/// `_UnmanagedNode` reference to their final node. This reference does not +/// keep the targeted node alive -- it is the use site's responsibility to +/// ensure that the path is still valid before calling most of its operations. +/// +/// Note: paths only have a direct reference to their final node. This means +/// that ascending to the parent node requires following the path from the root +/// node down. (Paths could also store references to every node alongside them +/// in a fixed-size array; this would speed up walking over the tree, but it +/// would considerably embiggen the size of the path construct.) @usableFromInline internal struct _UnsafePath { @usableFromInline @@ -139,10 +163,8 @@ extension _UnsafePath { /// Returns true if this path addresses an item in the tree; otherwise returns /// false. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @inlinable @inline(__always) internal var isOnItem: Bool { // Note: this may be true even if nodeSlot == itemCount (insertion paths). @@ -153,6 +175,9 @@ extension _UnsafePath { /// valid item. Such paths can represent the place of an item that might be /// inserted later; they do not occur while simply iterating over existing /// items. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal var isPlaceholder: Bool { _isItem && nodeSlot.value == node.itemCount } @@ -160,10 +185,8 @@ extension _UnsafePath { /// Returns true if this path addresses a node in the tree; otherwise returns /// false. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal var isOnChild: Bool { !_isItem && nodeSlot.value < node.childCount } @@ -171,14 +194,14 @@ extension _UnsafePath { /// Returns true if this path addresses an empty slot within a node in a tree; /// otherwise returns false. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal var isOnNodeEnd: Bool { !_isItem && nodeSlot.value == node.childCount } + /// Returns true if this path addresses an item on the leftmost branch of the + /// tree, or the empty slot at the end of an empty tree. @inlinable internal var isOnLeftmostItem: Bool { // We are on the leftmost item in the tree if we are currently @@ -191,25 +214,25 @@ extension _UnsafePath { /// Returns an unmanaged reference to the child node this path is currently /// addressing. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal var currentChild: _UnmanagedNode { assert(isOnChild) return node.unmanagedChild(at: nodeSlot) } + /// Returns the chid slot in this path corresponding to the specified level. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal func childSlot(at level: _Level) -> _Slot { assert(level < self.level) return ancestors[level] } /// Returns the slot of the currently addressed item. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @inlinable @inline(__always) internal var currentItemSlot: _Slot { assert(isOnItem) @@ -221,10 +244,8 @@ extension _UnsafePath { /// Positions this path on the item with the specified slot within its /// current node. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @inlinable internal mutating func selectItem(at slot: _Slot) { // As a special exception, this allows slot to equal the item count. @@ -238,10 +259,8 @@ extension _UnsafePath { /// Positions this path on the child with the specified slot within its /// current node, without descending into it. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @inlinable internal mutating func selectChild(at slot: _Slot) { // As a special exception, this allows slot to equal the child count. @@ -253,10 +272,8 @@ extension _UnsafePath { /// Positions this path on the empty slot at the end of its current node. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @usableFromInline @_effects(releasenone) internal mutating func selectEnd() { @@ -266,13 +283,11 @@ extension _UnsafePath { /// Descend onto the first path within the currently selected child. /// (Either the first item if it exists, or the first child. If the child - /// is an empty node (which should not happen in a hash tree), then this + /// is an empty node (which should not happen in a valid hash tree), then this /// selects the empty slot at the end of it. /// - /// - Note: This method needs to resolve the unmanaged node reference - /// that is stored in the path. It is up to the caller to ensure this will - /// never get called when the node is no longer valid; otherwise this will - /// trigger undefined behavior. + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @usableFromInline @_effects(releasenone) internal mutating func descend() { @@ -283,6 +298,14 @@ extension _UnsafePath { self.level = level.descend() } + /// Ascend to the nearest ancestor for which the `test` predicate returns + /// true. Because paths do not contain references to every node on them, + /// you need to manually supply a valid reference to the root node. This + /// method visits every node between the root and the current final node on + /// the path. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal mutating func ascendToNearestAncestor( under root: _RawNode, where test: (_UnmanagedNode, _Slot) -> Bool @@ -303,7 +326,13 @@ extension _UnsafePath { self = best return true } - + /// Ascend to the parent node of this path. Because paths do not contain + /// references to every node on them, you need to manually supply a valid + /// reference to the root node. This method visits every node + /// between the root and the current final node on the path. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal mutating func ascend(under root: _RawNode) { assert(!self.level.isAtRoot) var n = root.unmanaged @@ -324,6 +353,13 @@ extension _UnsafePath { } extension _UnsafePath { + /// Given a path that is on an item, advance it to the next item within its + /// current node, and return true. If there is no next item, position the path + /// on the first child, and return false. If there is no children, position + /// the path on the node's end position, and return false. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. mutating func selectNextItem() -> Bool { assert(isOnItem) nodeSlot = nodeSlot.next() @@ -333,6 +369,12 @@ extension _UnsafePath { return false } + /// Given a path that is on a child node, advance it to the next child within + /// its current node, and return true. If there is no next child, position + /// the path on the node's end position, and return false. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. mutating func selectNextChild() -> Bool { assert(!isOnItem) let childEnd = node.childEnd @@ -343,6 +385,13 @@ extension _UnsafePath { } extension _UnsafePath { + /// If this path addresses a child node, descend into the leftmost item + /// within the subtree under it (i.e., the first item that would be visited + /// by a preorder walk within that subtree). Do nothing if the path addresses + /// an item or the end position. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. @usableFromInline @_effects(releasenone) internal mutating func descendToLeftMostItem() { @@ -351,6 +400,13 @@ extension _UnsafePath { } } + /// Given a path addressing a child node, descend into the rightmost item + /// within the subtree under it (i.e., the last item that would be visited + /// by a preorder walk within that subtree). Do nothing if the path addresses + /// an item or the end position. + /// + /// - Note: It is undefined behavior to call this on a path that is no longer + /// valid. internal mutating func descendToRightMostItem() { assert(isOnChild) while true { @@ -364,6 +420,9 @@ extension _UnsafePath { selectItem(at: itemEnd.previous()) } + /// Find the next item in a preorder walk in the tree following the currently + /// addressed item, and return true. Return false and do nothing if the + /// path does not currently address an item. @usableFromInline @_effects(releasenone) internal mutating func findSuccessorItem(under root: _RawNode) -> Bool { @@ -388,6 +447,9 @@ extension _UnsafePath { return true } + /// Find the previous item in a preorder walk in the tree preceding the + /// currently addressed position, and return true. + /// Return false if there is no previous item. @usableFromInline @_effects(releasenone) internal mutating func findPredecessorItem(under root: _RawNode) -> Bool { @@ -427,6 +489,11 @@ extension _UnsafePath { } extension _RawNode { + /// Return the integer position of the item addressed by the given path + /// within a preorder walk of the tree. If the path addresses the end + /// position, then return the number of items in the tree. + /// + /// This method must only be called on the root node. internal func preorderPosition( _ level: _Level, of path: _UnsafePath ) -> Int { @@ -446,6 +513,10 @@ extension _RawNode { return path.nodeSlot.value } + /// Return the number of steps between two paths within a preorder walk of the + /// tree. The two paths must not address a child node. + /// + /// This method must only be called on the root node. @usableFromInline @_effects(releasenone) internal func distance( @@ -534,6 +605,8 @@ extension _UnmanagedNode { } extension _Node { + /// Return the path to the given key in this tree if it exists; otherwise + /// return nil. @inlinable internal func path( to key: Key, hash: _Hash From 1324c4e8635273fb0277544fc640ed525155061c Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Fri, 16 Sep 2022 16:03:45 -0700 Subject: [PATCH 12/18] [OrderedDictionary] Implement index invalidation --- .../PersistentDictionary+Collection.swift | 5 ++++ .../PersistentDictionary.swift | 26 +++++++++---------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift index 125ded374..32f1b8abc 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Collection.swift @@ -94,6 +94,11 @@ extension PersistentDictionary: BidirectionalCollection { _root.isIdentical(to: i._root) && i._version == self._version } + @inlinable @inline(__always) + internal mutating func _invalidateIndices() { + _version &+= 1 + } + /// Accesses the key-value pair at the specified position. @inlinable public subscript(i: Index) -> Element { diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index e4381f698..5211d01b8 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -80,10 +80,10 @@ extension PersistentDictionary { @inlinable public subscript(key: Key) -> Value? { get { - return _get(key) + _root.get(.top, key, _Hash(key)) } - mutating set(optionalValue) { - if let value = optionalValue { + mutating set { + if let value = newValue { updateValue(value, forKey: key) } else { removeValue(forKey: key) @@ -97,10 +97,11 @@ extension PersistentDictionary { default defaultValue: @autoclosure () -> Value ) -> Value { get { - return _get(key) ?? defaultValue() + _root.get(.top, key, _Hash(key)) ?? defaultValue() + } + set { + updateValue(newValue, forKey: key) } - mutating set(value) { - updateValue(value, forKey: key) } } @@ -109,11 +110,6 @@ extension PersistentDictionary { _root.containsKey(.top, key, _Hash(key)) } - @inlinable - func _get(_ key: Key) -> Value? { - _root.get(.top, key, _Hash(key)) - } - /// Returns the index for the given key. @inlinable public func index(forKey key: Key) -> Index? { @@ -127,7 +123,9 @@ extension PersistentDictionary { public mutating func updateValue( _ value: __owned Value, forKey key: Key ) -> Value? { - return _root.updateValue(value, forKey: key, .top, _Hash(key)) + let old = _root.updateValue(value, forKey: key, .top, _Hash(key)) + if old == nil { _invalidateIndices() } + return old } // fluid/immutable API @@ -141,7 +139,9 @@ extension PersistentDictionary { @inlinable @discardableResult public mutating func removeValue(forKey key: Key) -> Value? { - _root.remove(key, .top, _Hash(key))?.value + let old = _root.remove(key, .top, _Hash(key))?.value + if old != nil { _invalidateIndices() } + return old } // fluid/immutable API From 1661bd431cd042724a088eb950d4543eb5e46f22 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Fri, 16 Sep 2022 16:05:32 -0700 Subject: [PATCH 13/18] [PersistentDictionary] Implement in-place mutations for defaulted subscript --- .../Node/_Node+Mutations.swift | 159 ++++++++++++++++-- .../Node/_Node+UnsafeHandle.swift | 12 ++ .../PersistentDictionary.swift | 9 + 3 files changed, 163 insertions(+), 17 deletions(-) diff --git a/Sources/PersistentCollections/Node/_Node+Mutations.swift b/Sources/PersistentCollections/Node/_Node+Mutations.swift index ff1438929..44756022d 100644 --- a/Sources/PersistentCollections/Node/_Node+Mutations.swift +++ b/Sources/PersistentCollections/Node/_Node+Mutations.swift @@ -12,13 +12,19 @@ // MARK: Node-level mutation operations extension _Node.UnsafeHandle { - /// Insert `item` at `slot` corresponding to `bucket`. + /// Make room for a new item at `slot` corresponding to `bucket`. /// There must be enough free space in the node to fit the new item. /// /// `itemMap` must not already reflect the insertion at the time this /// function is called. This method does not update `itemMap`. + /// + /// - Returns: an unsafe mutable pointer to uninitialized memory that is + /// ready to store the new item. It is the caller's responsibility to + /// initialize this memory. @inlinable - internal func _insertItem(_ item: __owned Element, at slot: _Slot) { + internal func _makeRoomForNewItem( + at slot: _Slot, _ bucket: _Bucket + ) -> UnsafeMutablePointer { assertMutable() let c = itemCount assert(slot.value <= c) @@ -33,7 +39,18 @@ extension _Node.UnsafeHandle { let prefix = c &- slot.value start.moveInitialize(from: start + 1, count: prefix) - (start + prefix).initialize(to: item) + + if bucket.isInvalid { + assert(isCollisionNode) + collisionCount &+= 1 + } else { + assert(!itemMap.contains(bucket)) + assert(!childMap.contains(bucket)) + itemMap.insert(bucket) + assert(itemMap.slot(of: bucket) == slot) + } + + return start + prefix } /// Insert `child` at `slot`. There must be enough free space in the node @@ -105,7 +122,7 @@ extension _Node.UnsafeHandle { } extension _Node { - @inlinable + @inlinable @inline(__always) internal mutating func insertItem( _ item: __owned Element, _ bucket: _Bucket ) { @@ -113,24 +130,14 @@ extension _Node { self.insertItem(item, at: slot, bucket) } - /// Insert `item` at `slot` corresponding to `bucket`. - /// There must be enough free space in the node to fit the new item. - @inlinable + @inlinable @inline(__always) internal mutating func insertItem( _ item: __owned Element, at slot: _Slot, _ bucket: _Bucket ) { self.count &+= 1 update { - $0._insertItem(item, at: slot) - if $0.isCollisionNode { - assert(bucket.isInvalid) - $0.collisionCount &+= 1 - } else { - assert(!$0.itemMap.contains(bucket)) - assert(!$0.childMap.contains(bucket)) - $0.itemMap.insert(bucket) - assert($0.itemMap.slot(of: bucket) == slot) - } + let p = $0._makeRoomForNewItem(at: slot, bucket) + p.initialize(to: item) } } @@ -395,3 +402,121 @@ extension _Node { } } } + +extension _Node { + @usableFromInline + @frozen + internal struct DefaultedValueUpdateState { + @usableFromInline + internal var item: Element + + @usableFromInline + internal var node: _UnmanagedNode + + @usableFromInline + internal var slot: _Slot + + @usableFromInline + internal var inserted: Bool + + @inlinable + internal init( + _ item: Element, + in node: _UnmanagedNode, + at slot: _Slot, + inserted: Bool + ) { + self.item = item + self.node = node + self.slot = slot + self.inserted = inserted + } + } + + @inlinable + internal mutating func prepareDefaultedValueUpdate( + _ key: Key, + _ defaultValue: () -> Value, + _ level: _Level, + _ hash: _Hash + ) -> DefaultedValueUpdateState { + let isUnique = self.isUnique() + let r = find(level, key, hash, forInsert: true) + switch r { + case .found(_, let slot): + ensureUnique(isUnique: isUnique) + return DefaultedValueUpdateState( + update { $0.itemPtr(at: slot).move() }, + in: unmanaged, + at: slot, + inserted: false) + + case .notFound(let bucket, let slot): + ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) + update { _ = $0._makeRoomForNewItem(at: slot, bucket) } + return DefaultedValueUpdateState( + (key, defaultValue()), + in: unmanaged, + at: slot, + inserted: true) + + case .newCollision(let bucket, let slot): + let existingHash = read { _Hash($0[item: slot].key) } + if hash == existingHash, hasSingletonItem { + // Convert current node to a collision node. + ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) + update { + $0.collisionCount = 1 + _ = $0._makeRoomForNewItem(at: _Slot(1), .invalid) + } + self.count &+= 1 + return DefaultedValueUpdateState( + (key, defaultValue()), + in: unmanaged, + at: _Slot(1), + inserted: true) + } + ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) + let existing = removeItem(at: slot, bucket) + var node = _Node( + level: level.descend(), + item1: existing, existingHash, + item2: (key, defaultValue()), hash) + insertChild(node, bucket) + return DefaultedValueUpdateState( + node.update { $0.itemPtr(at: _Slot(1)).move() }, + in: node.unmanaged, + at: _Slot(1), + inserted: true) + + case .expansion(let collisionHash): + self = Self( + level: level, + item1: (key, defaultValue()), hash, + child2: self, collisionHash) + return DefaultedValueUpdateState( + update { $0.itemPtr(at: .zero).move() }, + in: unmanaged, + at: .zero, + inserted: true) + + case .descend(_, let slot): + ensureUnique(isUnique: isUnique) + let res = update { + $0[child: slot].prepareDefaultedValueUpdate( + key, defaultValue, level.descend(), hash) + } + if res.inserted { count &+= 1 } + return res + } + } + + @inlinable + internal mutating func finalizeDefaultedValueUpdate( + _ state: __owned DefaultedValueUpdateState + ) { + UnsafeHandle.update(state.node) { + $0.itemPtr(at: state.slot).initialize(to: state.item) + } + } +} diff --git a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index 78a45c347..f41bc3bb7 100644 --- a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -85,6 +85,18 @@ extension _Node.UnsafeHandle { } } + @inlinable @inline(__always) + static func update( + _ node: _UnmanagedNode, + _ body: (Self) throws -> R + ) rethrows -> R { + try node.ref._withUnsafeGuaranteedRef { storage in + try storage.withUnsafeMutablePointers { header, elements in + try body(Self(header, UnsafeMutableRawPointer(elements), isMutable: true)) + } + } + } + @inlinable @inline(__always) static func update( _ storage: _RawStorage, diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index 5211d01b8..b38ff3b08 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -102,6 +102,15 @@ extension PersistentDictionary { set { updateValue(newValue, forKey: key) } + @inline(__always) // https://github.com/apple/swift-collections/issues/164 + _modify { + var state = _root.prepareDefaultedValueUpdate( + key, defaultValue, .top, _Hash(key)) + if state.inserted { _invalidateIndices() } + defer { + _root.finalizeDefaultedValueUpdate(state) + } + yield &state.item.value } } From 3b7d241e150b3fb3c2e6a56dcce9a62e9700268a Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Fri, 16 Sep 2022 16:05:50 -0700 Subject: [PATCH 14/18] [PersistentDictionary] Add some docs --- .../PersistentDictionary.swift | 212 ++++++++++++++++++ 1 file changed, 212 insertions(+) diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index b38ff3b08..97bdf7ae0 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -77,6 +77,66 @@ extension PersistentDictionary { self.init(uniqueKeysWithValues: zip(keys, values)) } + /// Accesses the value associated with the given key for reading and writing. + /// + /// This *key-based* subscript returns the value for the given key if the key + /// is found in the dictionary, or `nil` if the key is not found. + /// + /// The following example creates a new dictionary and prints the value of a + /// key found in the dictionary (`"Coral"`) and a key not found in the + /// dictionary (`"Cerise"`). + /// + /// var hues: PersistentDictionary = ["Heliotrope": 296, "Coral": 16, "Aquamarine": 156] + /// print(hues["Coral"]) + /// // Prints "Optional(16)" + /// print(hues["Cerise"]) + /// // Prints "nil" + /// + /// When you assign a value for a key and that key already exists, the + /// dictionary overwrites the existing value. If the dictionary doesn't + /// contain the key, the key and value are added as a new key-value pair. + /// + /// Inserting a new key-value pair into a dictionary invalidates all + /// previously returned indices to it. Updating the value of an existing key + /// does not invalidate any indices. + /// + /// Here, the value for the key `"Coral"` is updated from `16` to `18` and a + /// new key-value pair is added for the key `"Cerise"`. + /// + /// hues["Coral"] = 18 + /// print(hues["Coral"]) + /// // Prints "Optional(18)" + /// + /// hues["Cerise"] = 330 + /// print(hues["Cerise"]) + /// // Prints "Optional(330)" + /// + /// If you assign `nil` as the value for the given key, the dictionary + /// removes that key and its associated value. Removing an existing key-value + /// pair in the dictionary invalidates all previously returned indices to it. + /// Removing a key that doesn't exist does not invalidate any indices. + /// + /// In the following example, the key-value pair for the key `"Aquamarine"` + /// is removed from the dictionary by assigning `nil` to the key-based + /// subscript. + /// + /// hues["Aquamarine"] = nil + /// print(hues) + /// // Prints "["Coral": 18, "Heliotrope": 296, "Cerise": 330]" + /// + /// - Parameter key: The key to find in the dictionary. + /// + /// - Returns: The value associated with `key` if `key` is in the dictionary; + /// otherwise, `nil`. + /// + /// - Complexity: Traversing the hash tree to find a key requires visiting + /// O(log(`count`)) nodes. Looking up and updating values in the + /// dictionary through this subscript has an expected complexity of O(1) + /// hashing/comparison operations on average, if `Key` implements + /// high-quality hashing. + /// + /// When used to mutate dictionary values with shared storage, this + /// operation is expected to copy at most O(log(`count`)) items. @inlinable public subscript(key: Key) -> Value? { get { @@ -91,6 +151,73 @@ extension PersistentDictionary { } } + /// Accesses the value with the given key. If the dictionary doesn't contain + /// the given key, accesses the provided default value as if the key and + /// default value existed in the dictionary. + /// + /// Use this subscript when you want either the value for a particular key + /// or, when that key is not present in the dictionary, a default value. This + /// example uses the subscript with a message to use in case an HTTP response + /// code isn't recognized: + /// + /// var responseMessages: PersistentDictionary = [ + /// 200: "OK", + /// 403: "Access forbidden", + /// 404: "File not found", + /// 500: "Internal server error"] + /// + /// let httpResponseCodes = [200, 403, 301] + /// for code in httpResponseCodes { + /// let message = responseMessages[code, default: "Unknown response"] + /// print("Response \(code): \(message)") + /// } + /// // Prints "Response 200: OK" + /// // Prints "Response 403: Access forbidden" + /// // Prints "Response 301: Unknown response" + /// + /// When a dictionary's `Value` type has value semantics, you can use this + /// subscript to perform in-place operations on values in the dictionary. + /// The following example uses this subscript while counting the occurrences + /// of each letter in a string: + /// + /// let message = "Hello, Elle!" + /// var letterCounts: PersistentDictionary = [:] + /// for letter in message { + /// letterCounts[letter, default: 0] += 1 + /// } + /// // letterCounts == ["H": 1, "e": 2, "l": 4, "o": 1, ...] + /// + /// When `letterCounts[letter, defaultValue: 0] += 1` is executed with a + /// value of `letter` that isn't already a key in `letterCounts`, the + /// specified default value (`0`) is returned from the subscript, + /// incremented, and then added to the dictionary under that key. + /// + /// Inserting a new key-value pair invalidates all existing indices in the + /// dictionary. Updating the value of an existing key does not invalidate + /// any indices. + /// + /// - Note: Do not use this subscript to modify dictionary values if the + /// dictionary's `Value` type is a class. In that case, the default value + /// and key are not written back to the dictionary after an operation. (For + /// a variant of this operation that supports this usecase, see + /// `updateValue(forKey:default:_:)`.) + /// + /// - Parameters: + /// - key: The key the look up in the dictionary. + /// - defaultValue: The default value to use if `key` doesn't exist in the + /// dictionary. + /// + /// - Returns: The value associated with `key` in the dictionary; otherwise, + /// `defaultValue`. + /// + /// - Complexity: Traversing the hash tree to find a key requires visiting + /// O(log(`count`)) nodes. Looking up and updating values in the + /// dictionary through this subscript has an expected complexity of O(1) + /// hashing/comparison operations on average, if `Key` implements + /// high-quality hashing. + /// + /// When used to mutate dictionary values with shared storage, this + /// operation is expected to copy at most O(log(`count`)) items. @inlinable public subscript( key: Key, @@ -127,6 +254,55 @@ extension PersistentDictionary { return Index(_root: _root.unmanaged, version: _version, path: path) } + /// Updates the value stored in the dictionary for the given key, or appends a + /// new key-value pair if the key does not exist. + /// + /// Use this method instead of key-based subscripting when you need to know + /// whether the new value supplants the value of an existing key. If the + /// value of an existing key is updated, `updateValue(_:forKey:)` returns + /// the original value. + /// + /// var hues: PersistentDictionary = [ + /// "Heliotrope": 296, + /// "Coral": 16, + /// "Aquamarine": 156] + /// + /// if let oldValue = hues.updateValue(18, forKey: "Coral") { + /// print("The old value of \(oldValue) was replaced with a new one.") + /// } + /// // Prints "The old value of 16 was replaced with a new one." + /// + /// If the given key is not present in the dictionary, this method appends the + /// key-value pair and returns `nil`. + /// + /// if let oldValue = hues.updateValue(330, forKey: "Cerise") { + /// print("The old value of \(oldValue) was replaced with a new one.") + /// } else { + /// print("No value was found in the dictionary for that key.") + /// } + /// // Prints "No value was found in the dictionary for that key." + /// + /// Inserting a new key-value pair invalidates all existing indices in the + /// dictionary. Updating the value of an existing key does not invalidate + /// any indices. + /// + /// - Parameters: + /// - value: The new value to add to the dictionary. + /// - key: The key to associate with `value`. If `key` already exists in + /// the dictionary, `value` replaces the existing associated value. If + /// `key` isn't already a key of the dictionary, the `(key, value)` pair + /// is added. + /// + /// - Returns: The value that was replaced, or `nil` if a new key-value pair + /// was added. + /// + /// - Complexity: Traversing the hash tree to find a key requires visiting + /// O(log(`count`)) nodes. Updating values in the has an expected + /// complexity of O(1) hashing/comparison operations on average, if + /// `Key` implements high-quality hashing. + /// + /// When used to mutate dictionary values with shared storage, this + /// operation is expected to copy at most O(log(`count`)) items. @inlinable @discardableResult public mutating func updateValue( @@ -145,6 +321,42 @@ extension PersistentDictionary { return copy } + /// Removes the given key and its associated value from the dictionary. + /// + /// If the key is found in the dictionary, this method returns the key's + /// associated value, and invalidates all previously returned indices. + /// + /// var hues: OrderedDictionary = [ + /// "Heliotrope": 296, + /// "Coral": 16, + /// "Aquamarine": 156] + /// if let value = hues.removeValue(forKey: "Coral") { + /// print("The value \(value) was removed.") + /// } + /// // Prints "The value 16 was removed." + /// + /// If the key isn't found in the dictionary, `removeValue(forKey:)` returns + /// `nil`. Removing a key that isn't in the dictionary does not invalidate + /// any indices. + /// + /// if let value = hues.removeValue(forKey: "Cerise") { + /// print("The value \(value) was removed.") + /// } else { + /// print("No value found for that key.") + /// } + /// // Prints "No value found for that key."" + /// + /// - Parameter key: The key to remove along with its associated value. + /// - Returns: The value that was removed, or `nil` if the key was not + /// present in the dictionary. + /// + /// - Complexity: Traversing the hash tree to find a key requires visiting + /// O(log(`count`)) nodes. Updating values in the has an expected + /// complexity of O(1) hashing/comparison operations on average, if + /// `Key` implements high-quality hashing. + /// + /// When used to mutate dictionary values with shared storage, this + /// operation is expected to copy at most O(log(`count`)) items. @inlinable @discardableResult public mutating func removeValue(forKey key: Key) -> Value? { From 57deed5f0fe51f7cf7d60058e4838b83c3fe5db1 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Sat, 17 Sep 2022 16:44:55 -0700 Subject: [PATCH 15/18] [PersistentDictionary] Implement in-place mutations # Conflicts: # Tests/PersistentCollections2Tests/PersistentMap Fixtures.swift --- .../Node/_Node+Initializers.swift | 139 +-- .../Node/_Node+Mutations.swift | 459 +++++++--- .../Node/_Node+UnsafeHandle.swift | 12 + .../Node/_UnsafePath.swift | 21 +- .../PersistentDictionary+Debugging.swift | 6 + .../PersistentDictionary.swift | 26 +- .../PersistentCollections Fixtures.swift | 111 ++- .../PersistentCollections Tests.swift | 790 +++++++++++++++++- .../Utilities.swift | 156 ++++ 9 files changed, 1506 insertions(+), 214 deletions(-) diff --git a/Sources/PersistentCollections/Node/_Node+Initializers.swift b/Sources/PersistentCollections/Node/_Node+Initializers.swift index b783ecb34..32dc6cfd9 100644 --- a/Sources/PersistentCollections/Node/_Node+Initializers.swift +++ b/Sources/PersistentCollections/Node/_Node+Initializers.swift @@ -11,118 +11,147 @@ extension _Node { @inlinable - internal init(_collisions item1: Element, _ item2: Element) { - defer { _invariantCheck() } - self.init(collisionCapacity: 2) - self.count = 2 - update { + internal static func _collisionNode( + _ item1: Element, + _ inserter2: (UnsafeMutablePointer) -> Void + ) -> _Node { + var node = _Node(collisionCapacity: 2) + node.count = 2 + node.update { $0.collisionCount = 2 let byteCount = 2 * MemoryLayout.stride assert($0.bytesFree >= byteCount) $0.bytesFree &-= byteCount let items = $0.reverseItems items.initializeElement(at: 1, to: item1) - items.initializeElement(at: 0, to: item2) + inserter2(items.baseAddress.unsafelyUnwrapped) } + node._invariantCheck() + return node } @inlinable - internal init( - _items item1: Element, _ bucket1: _Bucket, - _ item2: Element, _ bucket2: _Bucket - ) { - defer { _invariantCheck() } + internal static func _regularNode( + _ item1: Element, + _ bucket1: _Bucket, + _ inserter2: (UnsafeMutablePointer) -> Void, + _ bucket2: _Bucket + ) -> (node: _Node, slot1: _Slot, slot2: _Slot) { assert(bucket1 != bucket2) - self.init(itemCapacity: 2) - self.count = 2 - update { + var node = _Node(itemCapacity: 2) + node.count = 2 + let (slot1, slot2) = node.update { $0.itemMap.insert(bucket1) $0.itemMap.insert(bucket2) $0.bytesFree &-= 2 * MemoryLayout.stride - let i = bucket1 < bucket2 ? 1 : 0 + let i1 = bucket1 < bucket2 ? 1 : 0 + let i2 = 1 &- i1 let items = $0.reverseItems - items.initializeElement(at: i, to: item1) - items.initializeElement(at: 1 &- i, to: item2) + items.initializeElement(at: i1, to: item1) + inserter2(items.baseAddress.unsafelyUnwrapped + i2) + return (_Slot(i2), _Slot(i1)) // Note: swapped } + node._invariantCheck() + return (node, slot1, slot2) } @inlinable - internal init(_child: _Node, _ bucket: _Bucket) { - defer { _invariantCheck() } - self.init(childCapacity: 1) - self.count = _child.count - update { + internal static func _regularNode( + _ child: _Node, _ bucket: _Bucket + ) -> _Node { + var node = _Node(childCapacity: 1) + node.count = child.count + node.update { $0.childMap.insert(bucket) $0.bytesFree &-= MemoryLayout<_Node>.stride - $0.childPtr(at: .zero).initialize(to: _child) + $0.childPtr(at: .zero).initialize(to: child) } + node._invariantCheck() + return node } @inlinable - internal init( - _item: Element, _ itemBucket: _Bucket, - child: _Node, _ childBucket: _Bucket - ) { - defer { _invariantCheck() } + internal static func _regularNode( + _ inserter: (UnsafeMutablePointer) -> Void, + _ itemBucket: _Bucket, + _ child: _Node, + _ childBucket: _Bucket + ) -> _Node { assert(itemBucket != childBucket) - self.init(itemCapacity: 1, childCapacity: 1) - self.count = child.count + 1 - update { + var node = _Node(itemCapacity: 1, childCapacity: 1) + node.count = child.count + 1 + node.update { $0.itemMap.insert(itemBucket) $0.childMap.insert(childBucket) $0.bytesFree &-= MemoryLayout.stride + MemoryLayout<_Node>.stride - $0.itemPtr(at: .zero).initialize(to: _item) + inserter($0.itemPtr(at: .zero)) $0.childPtr(at: .zero).initialize(to: child) } + node._invariantCheck() + return node } } extension _Node { @inlinable - internal init( + internal static func build( level: _Level, item1: Element, _ hash1: _Hash, - item2: Element, + item2 inserter2: (UnsafeMutablePointer) -> Void, _ hash2: _Hash - ) { + ) -> (top: _Node, leaf: _UnmanagedNode, slot1: _Slot, slot2: _Slot) { if hash1 == hash2 { - self.init(_collisions: item1, item2) - } else { - let b1 = hash1[level] - let b2 = hash2[level] - if b1 == b2 { - let node = Self( - level: level.descend(), - item1: item1, hash1, - item2: item2, hash2) - self.init(_child: node, b1) - } else { - self.init(_items: item1, b1, item2, b2) - } + let top = _collisionNode(item1, inserter2) + return (top, top.unmanaged, _Slot(0), _Slot(1)) } + let r = _build( + level: level, item1: item1, hash1, item2: inserter2, hash2) + return (r.top, r.leaf, r.slot1, r.slot2) } @inlinable - internal init( + internal static func _build( level: _Level, item1: Element, _ hash1: _Hash, + item2 inserter2: (UnsafeMutablePointer) -> Void, + _ hash2: _Hash + ) -> (top: _Node, leaf: _UnmanagedNode, slot1: _Slot, slot2: _Slot) { + assert(hash1 != hash2) + let b1 = hash1[level] + let b2 = hash2[level] + guard b1 == b2 else { + let r = _regularNode(item1, b1, inserter2, b2) + return (r.node, r.node.unmanaged, r.slot1, r.slot2) + } + let r = _build( + level: level.descend(), + item1: item1, hash1, + item2: inserter2, hash2) + return (_regularNode(r.top, b1), r.leaf, r.slot1, r.slot2) + } + + @inlinable + internal static func build( + level: _Level, + item1 inserter1: (UnsafeMutablePointer) -> Void, + _ hash1: _Hash, child2: _Node, _ hash2: _Hash - ) { + ) -> (top: _Node, leaf: _UnmanagedNode, slot1: _Slot, slot2: _Slot) { assert(child2.isCollisionNode) assert(hash1 != hash2) let b1 = hash1[level] let b2 = hash2[level] if b1 == b2 { - let node = Self( + let node = build( level: level.descend(), - item1: item1, hash1, + item1: inserter1, hash1, child2: child2, hash2) - self.init(_child: node, b1) - } else { - self.init(_item: item1, hash1[level], child: child2, hash2[level]) + return (_regularNode(node.top, b1), node.leaf, node.slot1, node.slot2) } + let node = _regularNode(inserter1, hash1[level], child2, hash2[level]) + return (node, node.unmanaged, .zero, .zero) } } diff --git a/Sources/PersistentCollections/Node/_Node+Mutations.swift b/Sources/PersistentCollections/Node/_Node+Mutations.swift index 44756022d..3cbafc293 100644 --- a/Sources/PersistentCollections/Node/_Node+Mutations.swift +++ b/Sources/PersistentCollections/Node/_Node+Mutations.swift @@ -82,7 +82,10 @@ extension _Node.UnsafeHandle { /// `itemMap` must not yet reflect the removal at the time this /// function is called. This method does not update `itemMap`. @inlinable - internal func _removeItem(at slot: _Slot) -> Element { + internal func _removeItem( + at slot: _Slot, + by remover: (UnsafeMutablePointer) -> R + ) -> R { assertMutable() let c = itemCount assert(slot.value < c) @@ -95,9 +98,10 @@ extension _Node.UnsafeHandle { let prefix = c &- 1 &- slot.value let q = start + prefix - let item = q.move() - (start + 1).moveInitialize(from: start, count: prefix) - return item + defer { + (start + 1).moveInitialize(from: start, count: prefix) + } + return remover(q) } /// Remove and return the child at `slot`, increasing the amount of free @@ -159,19 +163,22 @@ extension _Node { } } - /// Remove and return the item at `slot`, increasing the amount of free + /// Remove the item at `slot`, increasing the amount of free /// space available in the node. + /// + /// The closure `remove` is called to perform the deinitialization of the + /// storage slot corresponding to the item to be removed. @inlinable - internal mutating func removeItem( - at slot: _Slot, _ bucket: _Bucket - ) -> Element { + internal mutating func removeItem( + at slot: _Slot, _ bucket: _Bucket, + by remover: (UnsafeMutablePointer) -> R + ) -> R { defer { _invariantCheck() } assert(count > 0) count &-= 1 return update { - let old = $0._removeItem(at: slot) + let old = $0._removeItem(at: slot, by: remover) if $0.isCollisionNode { - assert(bucket.isInvalid) assert(slot.value < $0.collisionCount) $0.collisionCount &-= 1 } else { @@ -208,7 +215,7 @@ extension _Node { count = 0 return update { assert($0.itemCount == 1 && $0.childCount == 0) - let old = $0._removeItem(at: .zero) + let old = $0._removeItem(at: .zero) { $0.move() } $0.itemMap = .empty $0.childMap = .empty return old @@ -258,27 +265,19 @@ extension _Node { insertItem((key, value), at: slot, bucket) return nil case .newCollision(let bucket, let slot): - let existingHash = read { _Hash($0[item: slot].key) } - if hash == existingHash, hasSingletonItem { - // Convert current node to a collision node. - ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) - update { $0.collisionCount = 1 } - insertItem((key, value), at: _Slot(1), .invalid) - } else { - ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) - let existing = removeItem(at: slot, bucket) - let node = _Node( - level: level.descend(), - item1: existing, existingHash, - item2: (key, value), hash) - insertChild(node, bucket) - } + _ = _insertNewCollision( + isUnique: isUnique, + level: level, + for: hash, + replacing: slot, bucket, + inserter: { $0.initialize(to: (key, value)) }) return nil case .expansion(let collisionHash): - self = Self( + self = _Node.build( level: level, - item1: (key, value), hash, - child2: self, collisionHash) + item1: { $0.initialize(to: (key, value)) }, hash, + child2: self, collisionHash + ).top return nil case .descend(_, let slot): ensureUnique(isUnique: isUnique) @@ -289,6 +288,36 @@ extension _Node { return old } } + + @inlinable + internal mutating func _insertNewCollision( + isUnique: Bool, + level: _Level, + for hash: _Hash, + replacing slot: _Slot, _ bucket: _Bucket, + inserter: (UnsafeMutablePointer) -> Void + ) -> (node: _UnmanagedNode, slot: _Slot) { + let existingHash = read { _Hash($0[item: slot].key) } + if hash == existingHash, hasSingletonItem { + // Convert current node to a collision node. + ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) + count &+= 1 + update { + $0.collisionCount = 1 + let p = $0._makeRoomForNewItem(at: _Slot(1), .invalid) + inserter(p) + } + return (unmanaged, _Slot(1)) + } + ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) + let existing = removeItem(at: slot, bucket) { $0.move() } + let r = _Node.build( + level: level.descend(), + item1: existing, existingHash, + item2: inserter, hash) + insertChild(r.top, bucket) + return (r.leaf, r.slot2) + } } extension _Node { @@ -310,14 +339,7 @@ extension _Node { let r = find(level, key, hash, forInsert: false) switch r { case .found(let bucket, let slot): - let old = removeItem(at: slot, bucket) - if isAtrophied { - self = removeSingletonChild() - } - if level.isAtRoot, isCollisionNode, hasSingletonItem { - self._convertToRegularNode() - } - return old + return _removeItemFromUniqueLeafNode(level, bucket, slot) { $0.move() } case .notFound, .newCollision, .expansion: return nil case .descend(let bucket, let slot): @@ -329,20 +351,50 @@ extension _Node { return (old, needsInlining) } guard old != nil else { return nil } - count &-= 1 - if needsInlining { - ensureUnique(isUnique: true, withFreeSpace: Self.spaceForInlinedChild) - var child = self.removeChild(at: slot, bucket) - let item = child.removeSingletonItem() - insertItem(item, bucket) - } - if isAtrophied { - self = removeSingletonChild() - } + _fixupUniqueAncestorAfterItemRemoval( + slot, { _ in bucket }, needsInlining: needsInlining) return old } } + @inlinable + internal mutating func _removeItemFromUniqueLeafNode( + _ level: _Level, + _ bucket: _Bucket, + _ slot: _Slot, + by remover: (UnsafeMutablePointer) -> R + ) -> R { + assert(isUnique()) + let result = removeItem(at: slot, bucket, by: remover) + if isAtrophied { + self = removeSingletonChild() + } + if level.isAtRoot, isCollisionNode, hasSingletonItem { + self._convertToRegularNode() + } + return result + } + + @inlinable + internal mutating func _fixupUniqueAncestorAfterItemRemoval( + _ slot: _Slot, + _ bucket: (inout Self) -> _Bucket, + needsInlining: Bool + ) { + assert(isUnique()) + count &-= 1 + if needsInlining { + ensureUnique(isUnique: true, withFreeSpace: Self.spaceForInlinedChild) + let bucket = bucket(&self) + var child = self.removeChild(at: slot, bucket) + let item = child.removeSingletonItem() + insertItem(item, bucket) + } + if isAtrophied { + self = removeSingletonChild() + } + } + @inlinable internal mutating func _convertToRegularNode() { assert(isCollisionNode && hasSingletonItem) @@ -361,44 +413,240 @@ extension _Node { let r = find(level, key, hash, forInsert: false) switch r { case .found(let bucket, let slot): - // Don't copy the node if we'd immediately discard it. - let willAtrophy = read { - $0.itemCount == 1 - && $0.childCount == 1 - && $0[child: .zero].isCollisionNode - } - if willAtrophy { - return read { ($0[child: .zero], $0[item: .zero]) } - } - var node = self.copy() - let old = node.removeItem(at: slot, bucket) - if level.isAtRoot, node.isCollisionNode, node.hasSingletonItem { - node._convertToRegularNode() - } - node._invariantCheck() - return (node, old) + return _removingItemFromLeaf(level, slot, bucket) case .notFound, .newCollision, .expansion: return nil case .descend(let bucket, let slot): let r = read { $0[child: slot].removing(key, level.descend(), hash) } - guard var r = r else { return nil } - if r.replacement.hasSingletonItem { - var node = copy(withFreeSpace: Self.spaceForInlinedChild) - _ = node.removeChild(at: slot, bucket) - let item = r.replacement.removeSingletonItem() - node.insertItem(item, bucket) - node._invariantCheck() - return (node, r.old) + guard let r = r else { return nil } + return ( + _fixedUpAncestorAfterItemRemoval(level, slot, bucket, r.replacement), + r.old) + } + } + + @inlinable + internal func _removingItemFromLeaf( + _ level: _Level, _ slot: _Slot, _ bucket: _Bucket + ) -> (replacement: _Node, old: Element) { + // Don't copy the node if we'd immediately discard it. + let willAtrophy = read { + $0.itemCount == 1 + && $0.childCount == 1 + && $0[child: .zero].isCollisionNode + } + if willAtrophy { + return read { (replacement: $0[child: .zero], old: $0[item: .zero]) } + } + var node = self.copy() + let old = node.removeItem(at: slot, bucket) { $0.move() } + if level.isAtRoot, node.isCollisionNode, node.hasSingletonItem { + node._convertToRegularNode() + } + node._invariantCheck() + return (node, old) + } + + @inlinable + internal func _fixedUpAncestorAfterItemRemoval( + _ level: _Level, + _ slot: _Slot, + _ bucket: _Bucket, + _ newChild: __owned _Node + ) -> _Node { + var newChild = newChild + if newChild.hasSingletonItem { + var node = copy(withFreeSpace: Self.spaceForInlinedChild) + _ = node.removeChild(at: slot, bucket) + let item = newChild.removeSingletonItem() + node.insertItem(item, bucket) + node._invariantCheck() + return node + } + if newChild.isCollisionNode && self.hasSingletonChild { + // Don't return an atrophied node. + return newChild + } + var node = copy() + node.update { $0[child: slot] = newChild } + node.count &-= 1 + node._invariantCheck() + return node + } +} + +extension _Node { + @inlinable + internal mutating func remove( + _ level: _Level, at path: _UnsafePath + ) -> Element { + defer { _invariantCheck() } + guard self.isUnique() else { + let r = removing(level, at: path) + self = r.replacement + return r.old + } + if level == path.level { + let slot = path.currentItemSlot + let bucket = read { $0.itemBucket(at: slot) } + return _removeItemFromUniqueLeafNode( + level, bucket, slot, by: { $0.move() }) + } + let slot = path.childSlot(at: level) + let (item, needsInlining) = update { + let child = $0.childPtr(at: slot) + let item = child.pointee.remove(level.descend(), at: path) + return (item, child.pointee.hasSingletonItem) + } + _fixupUniqueAncestorAfterItemRemoval( + slot, + { $0.read { $0.childMap.bucket(at: slot) } }, + needsInlining: needsInlining) + return item + } + + @inlinable + internal func removing( + _ level: _Level, at path: _UnsafePath + ) -> (replacement: _Node, old: Element) { + if level == path.level { + let slot = path.currentItemSlot + let bucket = read { $0.itemBucket(at: slot) } + return _removingItemFromLeaf(level, slot, bucket) + } + let slot = path.childSlot(at: level) + let (bucket, r) = read { + ($0.childMap.bucket(at: slot), + $0[child: slot].removing(level.descend(), at: path)) + } + return ( + _fixedUpAncestorAfterItemRemoval(level, slot, bucket, r.replacement), + r.old + ) + } +} + +extension _Node { + @usableFromInline + @frozen + internal struct ValueUpdateState { + @usableFromInline + internal var key: Key + + @usableFromInline + internal var value: Value? + + @usableFromInline + internal let hash: _Hash + + @usableFromInline + internal var path: _UnsafePath + + @usableFromInline + internal var found: Bool + + @inlinable + internal init( + _ key: Key, + _ hash: _Hash, + _ path: _UnsafePath + ) { + self.key = key + self.value = nil + self.hash = hash + self.path = path + self.found = false + } + } + + @inlinable + internal mutating func prepareValueUpdate( + _ key: Key, + _ hash: _Hash + ) -> ValueUpdateState { + var state = ValueUpdateState(key, hash, _UnsafePath(root: raw)) + _prepareValueUpdate(&state) + return state + } + + @inlinable + internal mutating func _prepareValueUpdate( + _ state: inout ValueUpdateState + ) { + // This doesn't make room for a new item if the key doesn't already exist + // but it does ensure that all parent nodes along its eventual path are + // uniquely held. + // + // If the key already exists, we ensure uniqueness for its node and extract + // its item but otherwise leave the tree as it was. + let isUnique = self.isUnique() + let r = find(state.path.level, state.key, state.hash, forInsert: true) + switch r { + case .found(_, let slot): + ensureUnique(isUnique: isUnique) + state.path.node = unmanaged + state.path.selectItem(at: slot) + state.found = true + (state.key, state.value) = update { $0.itemPtr(at: slot).move() } + + case .notFound(_, let slot): + state.path.selectItem(at: slot) + + case .newCollision(_, let slot): + state.path.selectItem(at: slot) + + case .expansion(_): + state.path.selectEnd() + + case .descend(_, let slot): + ensureUnique(isUnique: isUnique) + state.path.selectChild(at: slot) + state.path.descend() + update { $0[child: slot]._prepareValueUpdate(&state) } + } + } + + @inlinable + internal mutating func finalizeValueUpdate( + _ state: __owned ValueUpdateState + ) { + switch (state.found, state.value != nil) { + case (true, true): + // Fast path: updating an existing value. + UnsafeHandle.update(state.path.node) { + $0.itemPtr(at: state.path.currentItemSlot) + .initialize(to: (state.key, state.value.unsafelyUnwrapped)) } - if r.replacement.isCollisionNode && self.hasSingletonChild { - // Don't return an atrophied node. - return (r.replacement, r.old) + case (true, false): + // Removal + _finalizeRemoval(.top, state.hash, at: state.path) + case (false, true): + // Insertion + _ = updateValue( + state.value.unsafelyUnwrapped, forKey: state.key, .top, state.hash) + case (false, false): + // Noop + break + } + } + + @inlinable + internal mutating func _finalizeRemoval( + _ level: _Level, _ hash: _Hash, at path: _UnsafePath + ) { + assert(isUnique()) + if level == path.level { + _removeItemFromUniqueLeafNode( + level, hash[level], path.currentItemSlot, by: { _ in }) + } else { + let slot = path.childSlot(at: level) + let needsInlining = update { + let child = $0.childPtr(at: slot) + child.pointee._finalizeRemoval(level.descend(), hash, at: path) + return child.pointee.hasSingletonItem } - var node = copy() - node.update { $0[child: slot] = r.replacement } - node.count &-= 1 - node._invariantCheck() - return (node, r.old) + _fixupUniqueAncestorAfterItemRemoval( + slot, { _ in hash[level] }, needsInlining: needsInlining) } } } @@ -435,9 +683,9 @@ extension _Node { @inlinable internal mutating func prepareDefaultedValueUpdate( + _ level: _Level, _ key: Key, _ defaultValue: () -> Value, - _ level: _Level, _ hash: _Hash ) -> DefaultedValueUpdateState { let isUnique = self.isUnique() @@ -454,6 +702,7 @@ extension _Node { case .notFound(let bucket, let slot): ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) update { _ = $0._makeRoomForNewItem(at: slot, bucket) } + self.count &+= 1 return DefaultedValueUpdateState( (key, defaultValue()), in: unmanaged, @@ -461,50 +710,36 @@ extension _Node { inserted: true) case .newCollision(let bucket, let slot): - let existingHash = read { _Hash($0[item: slot].key) } - if hash == existingHash, hasSingletonItem { - // Convert current node to a collision node. - ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewItem) - update { - $0.collisionCount = 1 - _ = $0._makeRoomForNewItem(at: _Slot(1), .invalid) - } - self.count &+= 1 - return DefaultedValueUpdateState( - (key, defaultValue()), - in: unmanaged, - at: _Slot(1), - inserted: true) - } - ensureUnique(isUnique: isUnique, withFreeSpace: Self.spaceForNewCollision) - let existing = removeItem(at: slot, bucket) - var node = _Node( - level: level.descend(), - item1: existing, existingHash, - item2: (key, defaultValue()), hash) - insertChild(node, bucket) + let r = _insertNewCollision( + isUnique: isUnique, + level: level, + for: hash, + replacing: slot, bucket, + inserter: { _ in }) return DefaultedValueUpdateState( - node.update { $0.itemPtr(at: _Slot(1)).move() }, - in: node.unmanaged, - at: _Slot(1), + (key, defaultValue()), + in: r.node, + at: r.slot, inserted: true) case .expansion(let collisionHash): - self = Self( + let r = _Node.build( level: level, - item1: (key, defaultValue()), hash, - child2: self, collisionHash) + item1: { _ in }, hash, + child2: self, collisionHash + ) + self = r.top return DefaultedValueUpdateState( - update { $0.itemPtr(at: .zero).move() }, - in: unmanaged, - at: .zero, + (key, defaultValue()), + in: r.leaf, + at: r.slot1, inserted: true) case .descend(_, let slot): ensureUnique(isUnique: isUnique) let res = update { $0[child: slot].prepareDefaultedValueUpdate( - key, defaultValue, level.descend(), hash) + level.descend(), key, defaultValue, hash) } if res.inserted { count &+= 1 } return res diff --git a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift index f41bc3bb7..146a7cd4e 100644 --- a/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift +++ b/Sources/PersistentCollections/Node/_Node+UnsafeHandle.swift @@ -171,6 +171,12 @@ extension _Node.UnsafeHandle { _header.pointee.childCount } + @inlinable + internal func childBucket(at slot: _Slot) -> _Bucket { + guard !isCollisionNode else { return .invalid } + return childMap.bucket(at: slot) + } + @inlinable @inline(__always) internal var childEnd: _Slot { _header.pointee.childEnd @@ -209,6 +215,12 @@ extension _Node.UnsafeHandle { _header.pointee.itemCount } + @inlinable + internal func itemBucket(at slot: _Slot) -> _Bucket { + guard !isCollisionNode else { return .invalid } + return itemMap.bucket(at: slot) + } + @inlinable @inline(__always) internal var itemEnd: _Slot { _header.pointee.itemEnd diff --git a/Sources/PersistentCollections/Node/_UnsafePath.swift b/Sources/PersistentCollections/Node/_UnsafePath.swift index fd5d39c5c..67b38dc39 100644 --- a/Sources/PersistentCollections/Node/_UnsafePath.swift +++ b/Sources/PersistentCollections/Node/_UnsafePath.swift @@ -66,7 +66,7 @@ extension _UnsafePath { _ level: _Level, _ ancestors: _AncestorSlots, _ node: _UnmanagedNode, - _ childSlot: _Slot + childSlot: _Slot ) { assert(childSlot < node.childEnd) self.level = level @@ -75,6 +75,21 @@ extension _UnsafePath { self.nodeSlot = childSlot self._isItem = false } + + @inlinable + internal init( + _ level: _Level, + _ ancestors: _AncestorSlots, + _ node: _UnmanagedNode, + itemSlot: _Slot + ) { + assert(itemSlot < node.itemEnd) + self.level = level + self.ancestors = ancestors + self.node = node + self.nodeSlot = itemSlot + self._isItem = true + } } extension _UnsafePath: Equatable { @@ -225,6 +240,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. + @inlinable internal func childSlot(at level: _Level) -> _Slot { assert(level < self.level) return ancestors[level] @@ -317,7 +333,8 @@ extension _UnsafePath { while l < self.level { let slot = self.ancestors[l] if test(n, slot) { - best = _UnsafePath(l, self.ancestors.truncating(to: l), n, slot) + best = _UnsafePath( + l, self.ancestors.truncating(to: l), n, childSlot: slot) } n = n.unmanagedChild(at: slot) l = l.descend() diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift index 086e70f86..1206ec5f9 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary+Debugging.swift @@ -16,3 +16,9 @@ extension PersistentDictionary { _root.dump(iterationOrder: iterationOrder) } } + +extension PersistentDictionary { + public static var _maxDepth: Int { + _Level.limit + } +} diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index 97bdf7ae0..9f2d05b9e 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -30,11 +30,10 @@ public struct PersistentDictionary where Key: Hashable { @inlinable internal init(_new: _Node) { - self._root = _new // Ideally we would simply just generate a true random number, but the // memory address of the root node is a reasonable substitute. - let address = Unmanaged.passUnretained(_root.raw.storage).toOpaque() - self._version = UInt(bitPattern: address) + let address = Unmanaged.passUnretained(_new.raw.storage).toOpaque() + self.init(_root: _new, version: UInt(bitPattern: address)) } } @@ -142,13 +141,22 @@ extension PersistentDictionary { get { _root.get(.top, key, _Hash(key)) } - mutating set { + set { if let value = newValue { updateValue(value, forKey: key) } else { removeValue(forKey: key) } } + @inline(__always) // https://github.com/apple/swift-collections/issues/164 + _modify { + _invalidateIndices() + var state = _root.prepareValueUpdate(key, _Hash(key)) + defer { + _root.finalizeValueUpdate(state) + } + yield &state.value + } } /// Accesses the value with the given key. If the dictionary doesn't contain @@ -232,7 +240,7 @@ extension PersistentDictionary { @inline(__always) // https://github.com/apple/swift-collections/issues/164 _modify { var state = _root.prepareDefaultedValueUpdate( - key, defaultValue, .top, _Hash(key)) + .top, key, defaultValue, _Hash(key)) if state.inserted { _invalidateIndices() } defer { _root.finalizeDefaultedValueUpdate(state) @@ -372,5 +380,13 @@ extension PersistentDictionary { copy.removeValue(forKey: key) return copy } + + @inlinable + public mutating func remove(at index: Index) -> Element { + precondition(_isValid(index), "Invalid index") + precondition(index._path._isItem, "Can't remove item at end index") + _invalidateIndices() + return _root.remove(.top, at: index._path) + } } diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift b/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift index a91116dfb..c8240133d 100644 --- a/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift +++ b/Tests/PersistentCollectionsTests/PersistentCollections Fixtures.swift @@ -99,7 +99,7 @@ let _fixtures: KeyValuePairs = [ "36664", "36667", ], - "chain": [ + "chain-left": [ "0", "10", "110", @@ -107,6 +107,38 @@ let _fixtures: KeyValuePairs = [ "11110", "11111", ], + "chain-right": [ + "1", + "01", + "001", + "0001", + "00001", + "000001", + ], + "expansion0": [ + "00000001*3", + "00001", + ], + "expansion1": [ + "00000001*3", + "01", + "00001", + ], + "expansion2": [ + "11111111*3", + "10", + "11110", + ], + "expansion3": [ + "01", + "00001", + "00000001*3", + ], + "expansion4": [ + "10", + "11110", + "11111111*3", + ], "nested": [ "50", "51", @@ -176,7 +208,14 @@ func withEachFixture( let entry = TestContext.current.push("fixture: \(name)") defer { TestContext.current.pop(entry) } - let maxDepth = 15 // Larger than the actual maximum tree depth + let maxDepth = PersistentDictionary._maxDepth + + func normalized(_ path: String) -> String { + precondition(path.unicodeScalars.count < maxDepth) + let c = Swift.max(0, maxDepth - path.unicodeScalars.count) + return path.uppercased() + String(repeating: "0", count: c) + } + var paths: [String] = [] var seen: Set = [] for item in fixture { @@ -187,44 +226,53 @@ func withEachFixture( let p = String(item.unicodeScalars.prefix(upTo: i)) guard let count = Int(item.suffix(from: i).dropFirst(), radix: 10) else { fatalError("Invalid item: '\(item)'") } - path = p.appending(String(repeating: "0", count: maxDepth - p.unicodeScalars.count)) + path = normalized(p) paths.append(contentsOf: repeatElement(path, count: count)) } else { - path = item + path = normalized(item) paths.append(path) } - let normalized = path - .uppercased() - .appending(String(repeating: "0", count: maxDepth - path.unicodeScalars.count)) - if !seen.insert(normalized).inserted { + if !seen.insert(path).inserted { fatalError("Unexpected duplicate path: '\(path)'") } } + var seenPrefixes: Set = [] + var collidingPrefixes: Set = [] + for p in paths { + assert(p.count == maxDepth) + for i in p.indices { + let prefix = p[.. b[j] { return false } + a.formIndex(after: &i) + b.formIndex(after: &j) } - switch (a.isEmpty, b.isEmpty) { - case (true, true): return false // a == b - case (true, false): return true // a < b, like 44 < 443 - case (false, true): return false // a > b like 443 > 44 - case (false, false): break - } - if a.count == 1 && b.count > 1 { - return true // a < b, like 45 < 423 - } - if b.count == 1 && a.count > 1 { - return false // a > b, like 423 > 45 - } - return a.first! < b.first! + precondition(i == a.endIndex && j == b.endIndex) + return false } var items: [(key: RawCollider, value: Int)] = [] @@ -246,14 +294,17 @@ func withEachFixture( extension LifetimeTracker { func persistentDictionary( - for fixture: [(key: RawCollider, value: Int)] + for fixture: Fixture ) -> ( map: PersistentDictionary, LifetimeTracked>, ref: [(key: LifetimeTracked, value: LifetimeTracked)] ) { - let ref = fixture.map { (key, value) in + let ref = fixture.items.map { (key, value) in + (key: self.instance(for: key), value: self.instance(for: value)) + } + let ref2 = fixture.items.map { (key, value) in (key: self.instance(for: key), value: self.instance(for: value)) } - return (PersistentDictionary(uniqueKeysWithValues: ref), ref) + return (PersistentDictionary(uniqueKeysWithValues: ref2), ref) } } diff --git a/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift b/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift index e6fdcb426..a7f2610f3 100644 --- a/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift +++ b/Tests/PersistentCollectionsTests/PersistentCollections Tests.swift @@ -138,7 +138,6 @@ class PersistentDictionaryTests: CollectionTestCase { remaining -= 1 expectEqual(d.count, remaining) } - } func test_collisions() throws { @@ -252,17 +251,795 @@ class PersistentDictionaryTests: CollectionTestCase { func test_BidirectionalCollection_fixtures() { withEachFixture { fixture in withLifetimeTracking { tracker in - let (d, ref) = tracker.persistentDictionary(for: fixture.items) + let (d, ref) = tracker.persistentDictionary(for: fixture) checkBidirectionalCollection(d, expectedContents: ref, by: ==) } } } - func test_BidirectionalCollection_random() { + func test_BidirectionalCollection_random100() { let d = PersistentDictionary(uniqueKeys: 0 ..< 100, values: 0 ..< 100) checkBidirectionalCollection(d, expectedContents: Array(d), by: ==) } + func test_removeValueForKey_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withHiddenCopies(if: isShared, of: &d) { d in + let old = d.removeValue(forKey: ref[offset].key) + d._invariantCheck() + expectEqual(old, ref[offset].value) + ref.remove(at: offset) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_getter_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(0 ..< count, with: generator) + withEvery("key", in: 0 ..< count) { key in + let key = tracker.instance(for: generator.key(for: key)) + expectEqual(d[key], ref[key]) + } + expectNil(d[tracker.instance(for: generator.key(for: -1))]) + expectNil(d[tracker.instance(for: generator.key(for: count))]) + } + } + } + + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_getter_fixtures() { + withEachFixture { fixture in + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(for: fixture) + for (k, v) in ref { + expectEqual(d[k], v, "\(k)") + } + } + } + } + + func test_subscript_setter_update_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("key", in: 0 ..< count) { key in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary( + 0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: key)) + let value = tracker.instance(for: generator.value(for: -1)) + withHiddenCopies(if: isShared, of: &d) { d in + d[key] = value + ref[key] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 40 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_setter_update_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let replacement = tracker.instance(for: -1000) + withHiddenCopies(if: isShared, of: &d) { d in + let key = ref[offset].key + d[key] = replacement + ref[offset].value = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_setter_remove_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("key", in: 0 ..< count) { key in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, reference) = tracker.persistentDictionary(keys: 0 ..< count) + let key = tracker.instance(for: key) + withHiddenCopies(if: isShared, of: &d) { d in + d[key] = nil + reference.removeValue(forKey: key) + expectEqualDictionaries(d, reference) + } + } + } + } + } + } + let c = 40 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_setter_remove_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withHiddenCopies(if: isShared, of: &d) { d in + d[ref[offset].key] = nil + ref.remove(at: offset) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_setter_remove_fixtures_removeAll() { + withEachFixture { fixture in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withEvery("i", in: 0 ..< ref.count) { _ in + withHiddenCopies(if: isShared, of: &d) { d in + d[ref[0].key] = nil + ref.remove(at: 0) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_setter_insert_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + let keys = tracker.instances( + for: (0 ..< count).map { generator.key(for: $0) }) + let values = tracker.instances( + for: (0 ..< count).map { generator.value(for: $0) }) + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + withEvery("offset", in: 0 ..< count) { offset in + withHiddenCopies(if: isShared, of: &d) { d in + d[keys[offset]] = values[offset] + ref[keys[offset]] = values[offset] + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_setter_insert_fixtures() { + withEachFixture { fixture in + withEvery("seed", in: 0..<3) { seed in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + withEvery("i", in: 0 ..< fixture.items.count) { i in + withHiddenCopies(if: isShared, of: &d) { d in + let item = fixture.items[i] + let key = tracker.instance(for: item.key) + let value = tracker.instance(for: item.value) + d[key] = value + ref[key] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + } + + func test_subscript_setter_noop() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: -1)) + withHiddenCopies(if: isShared, of: &d) { d in + d[key] = nil + } + expectEqualDictionaries(d, ref) + } + } + } + } + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_modify_basics() { + func check(count: Int, generator: G) where G.Value == Int { + context.withTrace("count: \(count), generator: \(generator)") { + var d: PersistentDictionary = [:] + var ref: Dictionary = [:] + + // Insertions + withEvery("i", in: 0 ..< count) { i in + let key = generator.key(for: i) + let value = generator.value(for: i) + mutate(&d[key]) { v in + expectNil(v) + v = value + } + ref[key] = value + expectEqualDictionaries(d, ref) + } + + // Updates + withEvery("i", in: 0 ..< count) { i in + let key = generator.key(for: i) + let value = generator.value(for: i) + + mutate(&d[key]) { v in + expectEqual(v, value) + v! *= 2 + } + ref[key]! *= 2 + expectEqualDictionaries(d, ref) + } + + // Removals + withEvery("i", in: 0 ..< count) { i in + let key = generator.key(for: i) + let value = generator.value(for: i) + + mutate(&d[key]) { v in + expectEqual(v, 2 * value) + v = nil + } + ref[key] = nil + expectEqualDictionaries(d, ref) + } + } + } + + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 3, valueOffset: c)) + } + + func test_subscript_modify_update_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("key", in: 0 ..< count) { key in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary( + 0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: key)) + let replacement = tracker.instance(for: generator.value(for: -1)) + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[key]) { value in + expectNotNil(value) + value = replacement + } + ref[key] = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 50 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_modify_update_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let replacement = tracker.instance(for: -1000) + withHiddenCopies(if: isShared, of: &d) { d in + let key = ref[offset].key + mutate(&d[key]) { value in + expectNotNil(value) + value = replacement + } + ref[offset].value = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_modify_in_place() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let key = ref[offset].key + mutate(&d[key]) { value in + expectNotNil(value) + expectTrue(isKnownUniquelyReferenced(&value)) + } + } + } + } + } + + + + func test_subscript_modify_remove_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[ref[offset].key]) { value in + expectNotNil(value) + value = nil + } + ref.remove(at: offset) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_modify_remove_fixtures_removeAll() { + withEachFixture { fixture in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withEvery("i", in: 0 ..< ref.count) { i in + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[ref[0].key]) { value in + expectNotNil(value) + value = nil + } + ref.remove(at: 0) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_subscript_modify_insert_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + let keys = tracker.instances( + for: (0 ..< count).map { generator.key(for: $0) }) + let values = tracker.instances( + for: (0 ..< count).map { generator.value(for: $0) }) + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + withEvery("offset", in: 0 ..< count) { offset in + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[keys[offset]]) { value in + expectNil(value) + value = values[offset] + } + ref[keys[offset]] = values[offset] + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_subscript_modify_insert_fixtures() { + withEachFixture { fixture in + withEvery("seed", in: 0..<3) { seed in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + withEvery("i", in: 0 ..< fixture.items.count) { i in + withHiddenCopies(if: isShared, of: &d) { d in + let item = fixture.items[i] + let key = tracker.instance(for: item.key) + let value = tracker.instance(for: item.value) + mutate(&d[key]) { v in + expectNil(v) + v = value + } + ref[key] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + } + + func test_subscript_modify_noop_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: -1)) + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[key]) { value in + expectNil(value) + value = nil + } + expectEqualDictionaries(d, ref) + } + } + } + } + } + let c = 50 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_defaulted_subscript_basics() { + var d: PersistentDictionary = [:] + + expectEqual(d[1, default: 0], 0) + + d[1, default: 0] = 2 + expectEqual(d[1, default: 0], 2) + expectEqual(d[2, default: 0], 0) + + mutate(&d[2, default: 0]) { value in + expectEqual(value, 0) + value = 4 + } + expectEqual(d[2, default: 0], 4) + + mutate(&d[2, default: 0]) { value in + expectEqual(value, 4) + value = 6 + } + expectEqual(d[2, default: 0], 6) + } + + func test_defaulted_subscript_getter_fixtures() { + withEachFixture { fixture in + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(for: fixture) + let def = tracker.instance(for: -1) + for (k, v) in ref { + expectEqual(d[k, default: def], v, "\(k)") + } + } + } + } + + func test_defaulted_subscript_setter_update_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("key", in: 0 ..< count) { key in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary( + 0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: key)) + let value = tracker.instance(for: generator.value(for: -1)) + let def = tracker.instance(for: generator.value(for: -2)) + withHiddenCopies(if: isShared, of: &d) { d in + d[key, default: def] = value + ref[key, default: def] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 40 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_defaulted_subscript_setter_update_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let replacement = tracker.instance(for: -1000) + let def = tracker.instance(for: -1) + withHiddenCopies(if: isShared, of: &d) { d in + let key = ref[offset].key + d[key, default: def] = replacement + ref[offset].value = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_defaulted_subscript_setter_insert_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + let keys = tracker.instances( + for: (0 ..< count).map { generator.key(for: $0) }) + let values = tracker.instances( + for: (0 ..< count).map { generator.value(for: $0) }) + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + let def = tracker.instance(for: generator.value(for: -1000)) + withEvery("offset", in: 0 ..< count) { offset in + withHiddenCopies(if: isShared, of: &d) { d in + d[keys[offset], default: def] = values[offset] + ref[keys[offset]] = values[offset] + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_defaulted_subscript_setter_insert_fixtures() { + withEachFixture { fixture in + withEvery("seed", in: 0..<3) { seed in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + let def = tracker.instance(for: -1000) + withEvery("i", in: 0 ..< fixture.items.count) { i in + withHiddenCopies(if: isShared, of: &d) { d in + let item = fixture.items[i] + let key = tracker.instance(for: item.key) + let value = tracker.instance(for: item.value) + d[key, default: def] = value + ref[key] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + } + + func test_defaulted_subscript_modify_update_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("key", in: 0 ..< count) { key in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary( + 0 ..< count, with: generator) + let key = tracker.instance(for: generator.key(for: key)) + let replacement = tracker.instance(for: generator.value(for: -1)) + let def = tracker.instance(for: generator.value(for: -2)) + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[key, default: def]) { value in + expectNotEqual(value, def) + value = replacement + } + ref[key] = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 50 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_defaulted_subscript_modify_update_fixtures() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let replacement = tracker.instance(for: -1000) + let def = tracker.instance(for: -1) + withHiddenCopies(if: isShared, of: &d) { d in + let key = ref[offset].key + mutate(&d[key, default: def]) { value in + expectNotNil(value) + value = replacement + } + ref[offset].value = replacement + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + + func test_defaulted_subscript_modify_in_place() { + withEachFixture { fixture in + withEvery("offset", in: 0 ..< fixture.items.count) { offset in + withLifetimeTracking { tracker in + var (d, ref) = tracker.persistentDictionary(for: fixture) + let key = ref[offset].key + mutate(&d[key, default: tracker.instance(for: -1)]) { value in + expectNotNil(value) + expectTrue(isKnownUniquelyReferenced(&value)) + } + } + } + } + } + + func test_defaulted_subscript_modify_insert_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + let keys = tracker.instances( + for: (0 ..< count).map { generator.key(for: $0) }) + let values = tracker.instances( + for: (0 ..< count).map { generator.value(for: $0) }) + let def = tracker.instance(for: generator.value(for: -1000)) + print(def) + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + withEvery("offset", in: 0 ..< count) { offset in + withHiddenCopies(if: isShared, of: &d) { d in + mutate(&d[keys[offset], default: def]) { value in + expectEqual(value, def) + value = values[offset] + } + ref[keys[offset]] = values[offset] + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + let c = 100 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_defaulted_subscript_modify_insert_fixtures() { + withEachFixture { fixture in + withEvery("seed", in: 0..<3) { seed in + withEvery("isShared", in: [false, true]) { isShared in + withLifetimeTracking { tracker in + var d: PersistentDictionary, LifetimeTracked> = [:] + var ref: Dictionary, LifetimeTracked> = [:] + let def = tracker.instance(for: -1) + withEvery("i", in: 0 ..< fixture.items.count) { i in + withHiddenCopies(if: isShared, of: &d) { d in + let item = fixture.items[i] + let key = tracker.instance(for: item.key) + let value = tracker.instance(for: item.value) + mutate(&d[key, default: def]) { v in + expectEqual(v, def) + v = value + } + ref[key] = value + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + } + + + func test_indexForKey_data() { + func check(count: Int, generator: G) { + context.withTrace("count: \(count), generator: \(generator)") { + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(0 ..< count, with: generator) + withEvery("key", in: ref.keys) { key in + let index = d.index(forKey: key) + expectNotNil(index) { index in + expectEqual(d[index].key, key) + expectEqual(d[index].value, ref[key]) + } + } + } + } + } + let c = 50 + check(count: c, generator: IntDataGenerator(valueOffset: c)) + check(count: c, generator: ColliderDataGenerator(groups: 5, valueOffset: c)) + } + + func test_indexForKey_fixtures() { + withEachFixture { fixture in + withLifetimeTracking { tracker in + let (d, ref) = tracker.persistentDictionary(for: fixture) + withEvery("offset", in: ref.indices) { offset in + let key = ref[offset].key + let value = ref[offset].value + let index = d.index(forKey: key) + expectNotNil(index) { index in + expectEqual(d[index].key, key) + expectEqual(d[index].value, value) + } + } + } + } + } + + func test_removeAt_fixtures() { + withEachFixture { fixture in + withLifetimeTracking { tracker in + withEvery("isShared", in: [false, true]) { isShared in + var (d, ref) = tracker.persistentDictionary(for: fixture) + withEvery("i", in: ref.indices) { _ in + let (key, value) = ref.removeFirst() + let index = d.index(forKey: key) + expectNotNil(index) { index in + withHiddenCopies(if: isShared, of: &d) { d in + let (k, v) = d.remove(at: index) + expectEqual(k, key) + expectEqual(v, value) + expectEqualDictionaries(d, ref) + } + } + } + } + } + } + } + + // MARK: - + // func test_uniqueKeysWithValues_Dictionary() { // let items: Dictionary = [ // "zero": 0, @@ -562,13 +1339,6 @@ class PersistentDictionaryTests: CollectionTestCase { // } // } - func mutate( - _ value: inout T, - _ body: (inout T) throws -> R - ) rethrows -> R { - try body(&value) - } - // func test_subscript_modify_update() { // withEvery("count", in: 0 ..< 30) { count in // withEvery("offset", in: 0 ..< count) { offset in diff --git a/Tests/PersistentCollectionsTests/Utilities.swift b/Tests/PersistentCollectionsTests/Utilities.swift index b4e6ad0bf..bcab10aa9 100644 --- a/Tests/PersistentCollectionsTests/Utilities.swift +++ b/Tests/PersistentCollectionsTests/Utilities.swift @@ -29,3 +29,159 @@ extension LifetimeTracker { return (dictionary, keys, values) } } + +protocol DataGenerator { + associatedtype Key: Hashable + associatedtype Value: Equatable + + func key(for i: Int) -> Key + func value(for i: Int) -> Value +} + +struct IntDataGenerator: DataGenerator { + typealias Key = Int + typealias Value = Int + + let valueOffset: Int + + init(valueOffset: Int) { + self.valueOffset = valueOffset + } + + func key(for i: Int) -> Key { + i + } + + func value(for i: Int) -> Value { + i + valueOffset + } +} + +struct ColliderDataGenerator: DataGenerator { + typealias Key = Collider + typealias Value = Int + + let groups: Int + let valueOffset: Int + + init(groups: Int, valueOffset: Int) { + self.groups = groups + self.valueOffset = valueOffset + } + + func key(for i: Int) -> Key { + Collider(i, Hash(i % groups)) + } + + func value(for i: Int) -> Value { + i + valueOffset + } +} + +extension LifetimeTracker { + func persistentDictionary( + _ payloads: Payloads, + with generator: G + ) -> ( + map: PersistentDictionary, LifetimeTracked>, + expected: [LifetimeTracked: LifetimeTracked] + ) + where Payloads.Element == Int + { + typealias Key = LifetimeTracked + typealias Value = LifetimeTracked + func gen() -> [(Key, Value)] { + payloads.map { + (instance(for: generator.key(for: $0)), + instance(for: generator.value(for: $0))) + } + } + let map = PersistentDictionary(uniqueKeysWithValues: gen()) + let expected = Dictionary(uniqueKeysWithValues: gen()) + return (map, expected) + } + + func persistentDictionary( + keys: Keys + ) -> ( + map: PersistentDictionary, LifetimeTracked>, + expected: [LifetimeTracked: LifetimeTracked] + ) + where Keys.Element == Int + { + persistentDictionary(keys, with: IntDataGenerator(valueOffset: 100)) + } +} + +func _expectFailure( + _ diagnostic: String, + _ message: () -> String, + trapping: Bool, + file: StaticString, + line: UInt +) { + expectFailure( + """ + \(diagnostic) + \(message()) + """, + trapping: trapping, + file: file, line: line) +} + +func expectEqualDictionaries( + _ map: PersistentDictionary, + _ ref: [(key: Key, value: Value)], + _ message: @autoclosure () -> String = "", + trapping: Bool = false, + file: StaticString = #file, + line: UInt = #line +) { + expectEqualDictionaries( + map, Dictionary(uniqueKeysWithValues: ref), + message(), + trapping: trapping, + file: file, line: line) +} + +func expectEqualDictionaries( + _ map: PersistentDictionary, + _ dict: Dictionary, + _ message: @autoclosure () -> String = "", + trapping: Bool = false, + file: StaticString = #file, + line: UInt = #line +) { + var dict = dict + var seen: Set = [] + var mismatches: [(key: Key, map: Value?, dict: Value?)] = [] + + for (key, value) in map { + if !seen.insert(key).inserted { + mismatches.append((key, value, nil)) + } else { + let expected = dict.removeValue(forKey: key) + if value != expected { + mismatches.append((key, value, expected)) + } + } + } + for (key, value) in dict { + mismatches.append((key, nil, value)) + } + if !mismatches.isEmpty { + let msg = mismatches.lazy.map { k, m, d in + "\n \(k): \(m == nil ? "nil" : "\(m!)") vs \(d == nil ? "nil" : "\(d!)")" + }.joined(separator: "") + _expectFailure( + "\n\(mismatches.count) mismatches (actual vs expected):\(msg)", + message, trapping: trapping, file: file, line: line) + } +} + +func mutate( + _ value: inout T, + _ body: (inout T) throws -> R +) rethrows -> R { + try body(&value) +} From 2321b34d804160338096bde7da9cbb196044fb6f Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Sat, 17 Sep 2022 16:47:11 -0700 Subject: [PATCH 16/18] [PersistentCollections] Fix node sizing logic Base the allocation request on the currently occupied space, not the full capacity. The original code tended to lead to unnecessary doubling of the node size in each copy, until we exhaust the 32 bit capacity value. (Oops.) --- Sources/PersistentCollections/Node/_Node+Storage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/PersistentCollections/Node/_Node+Storage.swift b/Sources/PersistentCollections/Node/_Node+Storage.swift index 413ab5021..bae2cfa59 100644 --- a/Sources/PersistentCollections/Node/_Node+Storage.swift +++ b/Sources/PersistentCollections/Node/_Node+Storage.swift @@ -152,7 +152,7 @@ extension _Node { internal func copy(withFreeSpace space: Int = 0) -> _Node { assert(space >= 0) let capacity = read { - $0.byteCapacity &+ Swift.max(0, space &- $0.bytesFree) + $0.byteCapacity &- $0.bytesFree &+ space } var new = Self(byteCapacity: capacity) new.count = self.count From 5240bf6e0a02211ca23eaf2757cd8ce5bfe583ef Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Sat, 17 Sep 2022 17:19:56 -0700 Subject: [PATCH 17/18] =?UTF-8?q?[PersistentCollections]=20Reduce=20=5FBuc?= =?UTF-8?q?ket=E2=80=99s=20storage=20size?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../PersistentCollections/Node/_Bucket.swift | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/Sources/PersistentCollections/Node/_Bucket.swift b/Sources/PersistentCollections/Node/_Bucket.swift index aa9f54581..43aeb54eb 100644 --- a/Sources/PersistentCollections/Node/_Bucket.swift +++ b/Sources/PersistentCollections/Node/_Bucket.swift @@ -15,12 +15,23 @@ @frozen internal struct _Bucket { @usableFromInline - internal var value: UInt + internal var _value: UInt8 + + @inlinable @inline(__always) + internal init(_value: UInt8) { + assert(_value < _Bitmap.capacity || _value == .max) + self._value = _value + } +} + +extension _Bucket { + @inlinable @inline(__always) + internal var value: UInt { UInt(truncatingIfNeeded: _value) } @inlinable @inline(__always) internal init(_ value: UInt) { assert(value < _Bitmap.capacity || value == .max) - self.value = value + self._value = UInt8(truncatingIfNeeded: value) } } @@ -32,29 +43,29 @@ extension _Bucket { static var bitMask: UInt { UInt(bitPattern: _Bitmap.capacity) &- 1 } @inlinable @inline(__always) - static var invalid: _Bucket { _Bucket(.max) } + static var invalid: _Bucket { _Bucket(_value: .max) } @inlinable @inline(__always) - var isInvalid: Bool { value == .max } + var isInvalid: Bool { _value == .max } } extension _Bucket: Equatable { @inlinable @inline(__always) internal static func ==(left: Self, right: Self) -> Bool { - left.value == right.value + left._value == right._value } } extension _Bucket: Comparable { @inlinable @inline(__always) internal static func <(left: Self, right: Self) -> Bool { - left.value < right.value + left._value < right._value } } extension _Bucket: CustomStringConvertible { @usableFromInline internal var description: String { - String(value, radix: _Bitmap.capacity) + String(_value, radix: _Bitmap.capacity) } } From d45f264ef0dc625a37801ee6e05b764ccbef30b4 Mon Sep 17 00:00:00 2001 From: Karoy Lorentey Date: Sat, 17 Sep 2022 19:09:28 -0700 Subject: [PATCH 18/18] [PersistentDictionary] Fix index(forKey:) performance --- .../Node/_AncestorOffsets.swift | 7 ++ .../Node/_Node+Lookups.swift | 65 ++++++++++++++++--- .../PersistentCollections/Node/_RawNode.swift | 2 +- .../Node/_UnmanagedNode.swift | 2 +- .../Node/_UnsafePath.swift | 41 ++++++------ .../PersistentDictionary.swift | 4 +- 6 files changed, 86 insertions(+), 35 deletions(-) diff --git a/Sources/PersistentCollections/Node/_AncestorOffsets.swift b/Sources/PersistentCollections/Node/_AncestorOffsets.swift index 2a6f793a1..f686ec9a1 100644 --- a/Sources/PersistentCollections/Node/_AncestorOffsets.swift +++ b/Sources/PersistentCollections/Node/_AncestorOffsets.swift @@ -58,6 +58,13 @@ extension _AncestorSlots { } } + @inlinable @inline(__always) + internal func appending(_ slot: _Slot, at level: _Level) -> Self { + var result = self + result[level] = slot + return result + } + /// Clear the slot at the specified level, by setting it to zero. @inlinable internal mutating func clear(_ level: _Level) { diff --git a/Sources/PersistentCollections/Node/_Node+Lookups.swift b/Sources/PersistentCollections/Node/_Node+Lookups.swift index 112d32f2b..3ad13650d 100644 --- a/Sources/PersistentCollections/Node/_Node+Lookups.swift +++ b/Sources/PersistentCollections/Node/_Node+Lookups.swift @@ -11,6 +11,55 @@ // MARK: Node-level lookup operations +extension _Node { + @inlinable + internal func find( + _ level: _Level, _ key: Key, _ hash: _Hash + ) -> (descend: Bool, slot: _Slot)? { + read { $0.find(level, key, hash) } + } +} + +extension _Node.UnsafeHandle { + @inlinable + internal func find( + _ level: _Level, _ key: Key, _ hash: _Hash + ) -> (descend: Bool, slot: _Slot)? { + guard !isCollisionNode else { + let r = _findInCollision(level, key, hash) + guard r.code == 0 else { return nil } + return (false, r.slot) + } + let bucket = hash[level] + if itemMap.contains(bucket) { + let slot = itemMap.slot(of: bucket) + guard self[item: slot].key == key else { return nil } + return (false, slot) + } + if childMap.contains(bucket) { + let slot = childMap.slot(of: bucket) + return (true, slot) + } + return nil + } + + @inlinable @inline(never) + internal func _findInCollision( + _ level: _Level, _ key: Key, _ hash: _Hash + ) -> (code: Int, slot: _Slot, expansionHash: _Hash) { + assert(isCollisionNode) + if !level.isAtBottom { + let h = _Hash(self[item: .zero].key) + if h != hash { return (2, .zero, h) } + } + // Note: this searches the items in reverse insertion order. + guard let slot = reverseItems.firstIndex(where: { $0.key == key }) + else { return (1, self.itemEnd, _Hash(_value: 0)) } + return (0, _Slot(itemCount &- 1 &- slot), _Hash(_value: 0)) + } +} + + /// Represents the results of a lookup operation within a single node of a hash /// tree. This enumeration captures all of the different cases that need to be /// covered if we wanted to insert a new item into the tree. @@ -88,17 +137,15 @@ extension _Node.UnsafeHandle { _ level: _Level, _ key: Key, _ hash: _Hash, forInsert: Bool ) -> _FindResult { guard !isCollisionNode else { - if !level.isAtBottom { - let h = _Hash(self[item: .zero].key) - if h != hash { - return .expansion(h) - } + let r = _findInCollision(level, key, hash) + if r.code == 0 { + return .found(.invalid, r.slot) } - // Note: this searches the items in reverse insertion order. - guard let slot = reverseItems.firstIndex(where: { $0.key == key }) else { - return .notFound(.invalid, itemEnd) + if r.code == 1 { + return .notFound(.invalid, self.itemEnd) } - return .found(.invalid, _Slot(itemCount &- 1 &- slot)) + assert(r.code == 2) + return .expansion(r.expansionHash) } let bucket = hash[level] if itemMap.contains(bucket) { diff --git a/Sources/PersistentCollections/Node/_RawNode.swift b/Sources/PersistentCollections/Node/_RawNode.swift index d0f681272..9cbc9848d 100644 --- a/Sources/PersistentCollections/Node/_RawNode.swift +++ b/Sources/PersistentCollections/Node/_RawNode.swift @@ -41,7 +41,7 @@ extension _RawNode { } extension _RawNode { - @inline(__always) + @inlinable @inline(__always) internal var unmanaged: _UnmanagedNode { _UnmanagedNode(storage) } diff --git a/Sources/PersistentCollections/Node/_UnmanagedNode.swift b/Sources/PersistentCollections/Node/_UnmanagedNode.swift index 279c5a4de..1928686d2 100644 --- a/Sources/PersistentCollections/Node/_UnmanagedNode.swift +++ b/Sources/PersistentCollections/Node/_UnmanagedNode.swift @@ -23,7 +23,7 @@ internal struct _UnmanagedNode { @usableFromInline internal var ref: Unmanaged<_RawStorage> - @inlinable + @inlinable @inline(__always) internal init(_ storage: _RawStorage) { self.ref = .passUnretained(storage) } diff --git a/Sources/PersistentCollections/Node/_UnsafePath.swift b/Sources/PersistentCollections/Node/_UnsafePath.swift index 67b38dc39..d507d3437 100644 --- a/Sources/PersistentCollections/Node/_UnsafePath.swift +++ b/Sources/PersistentCollections/Node/_UnsafePath.swift @@ -50,8 +50,7 @@ internal struct _UnsafePath { @usableFromInline internal var _isItem: Bool - @usableFromInline - @_effects(releasenone) + @inlinable internal init(root: __shared _RawNode) { self.level = .top self.ancestors = .empty @@ -193,6 +192,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. + @inlinable internal var isPlaceholder: Bool { _isItem && nodeSlot.value == node.itemCount } @@ -202,6 +202,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. + @inlinable internal var isOnChild: Bool { !_isItem && nodeSlot.value < node.childCount } @@ -211,6 +212,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. + @inlinable internal var isOnNodeEnd: Bool { !_isItem && nodeSlot.value == node.childCount } @@ -231,6 +233,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. + @inlinable internal var currentChild: _UnmanagedNode { assert(isOnChild) return node.unmanagedChild(at: nodeSlot) @@ -304,8 +307,7 @@ extension _UnsafePath { /// /// - Note: It is undefined behavior to call this on a path that is no longer /// valid. - @usableFromInline - @_effects(releasenone) + @inlinable internal mutating func descend() { self.node = currentChild self.ancestors[level] = nodeSlot @@ -626,26 +628,21 @@ extension _Node { /// return nil. @inlinable internal func path( - to key: Key, hash: _Hash - ) -> (found: Bool, path: _UnsafePath) { - var path = _UnsafePath(root: raw) + to key: Key, _ hash: _Hash + ) -> _UnsafePath? { + var node = unmanaged + var level: _Level = .top + var ancestors: _AncestorSlots = .empty while true { - let r = UnsafeHandle.read(path.node) { - $0.find(path.level, key, hash, forInsert: false) - } - switch r { - case .found(_, let slot): - path.selectItem(at: slot) - return (true, path) - case .notFound(_, let slot), .newCollision(_, let slot): - path.selectItem(at: slot) - return (false, path) - case .expansion: - return (false, path) - case .descend(_, let slot): - path.selectChild(at: slot) - path.descend() + let r = UnsafeHandle.read(node) { $0.find(level, key, hash) } + guard let r = r else { break } + guard r.descend else { + return _UnsafePath(level, ancestors, node, itemSlot: r.slot) } + node = node.unmanagedChild(at: r.slot) + ancestors[level] = r.slot + level = level.descend() } + return nil } } diff --git a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift index 9f2d05b9e..768df3bd3 100644 --- a/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift +++ b/Sources/PersistentCollections/PersistentDictionary/PersistentDictionary.swift @@ -257,8 +257,8 @@ extension PersistentDictionary { /// Returns the index for the given key. @inlinable public func index(forKey key: Key) -> Index? { - let (found, path) = _root.path(to: key, hash: _Hash(key)) - guard found else { return nil } + guard let path = _root.path(to: key, _Hash(key)) + else { return nil } return Index(_root: _root.unmanaged, version: _version, path: path) }