-
Notifications
You must be signed in to change notification settings - Fork 356
Let deque have its capacity shrunk #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,10 @@ extension Deque._Storage { | |
| #else | ||
| let capacity = $0.capacity | ||
| #endif | ||
| return _DequeBufferHeader(capacity: capacity, count: 0, startSlot: .zero) | ||
| return _DequeBufferHeader(capacity: capacity, | ||
| requestedCapacity: minimumCapacity, | ||
| count: 0, | ||
| startSlot: .zero) | ||
| }) | ||
| self.init(_buffer: _Buffer(unsafeBufferObject: object)) | ||
| } | ||
|
|
@@ -85,6 +88,12 @@ extension Deque._Storage { | |
| _buffer.withUnsafeMutablePointerToHeader { $0.pointee.capacity } | ||
| } | ||
|
|
||
| @inlinable | ||
| @inline(__always) | ||
| internal var requestedCapacity: Int { | ||
| _buffer.withUnsafeMutablePointerToHeader { $0.pointee.requestedCapacity } | ||
| } | ||
|
|
||
| @inlinable | ||
| @inline(__always) | ||
| internal var count: Int { | ||
|
|
@@ -94,8 +103,7 @@ extension Deque._Storage { | |
| @inlinable | ||
| @inline(__always) | ||
| internal var startSlot: _DequeSlot { | ||
| _buffer.withUnsafeMutablePointerToHeader { $0.pointee.startSlot | ||
| } | ||
| _buffer.withUnsafeMutablePointerToHeader { $0.pointee.startSlot } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -170,6 +178,11 @@ extension Deque._Storage { | |
| minimumCapacity) | ||
| } | ||
|
|
||
| @usableFromInline | ||
| internal func _reduceCapacity(to targetCapacity: Int) -> Int { | ||
| return Swift.max(0, Swift.min(requestedCapacity, targetCapacity)) | ||
| } | ||
|
|
||
| /// Ensure that we have a uniquely referenced buffer with enough space to | ||
| /// store at least `minimumCapacity` elements. | ||
| /// | ||
|
|
@@ -211,4 +224,27 @@ extension Deque._Storage { | |
| } | ||
| } | ||
| } | ||
|
|
||
| @inlinable | ||
| @inline(__always) | ||
| internal mutating func shrink(targetCapacity: Int) { | ||
| if count > targetCapacity { return } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, this raises a question: for a I suppose that instead of merely comparing directly against the count and capacity, we would want to only allow reallocation when the resulting capacity delta is "large enough" -- whatever that means. (An absolute cutoff? A percentage of existing capacity? Perhaps a combination of both. Probably the percentage value should be somewhat correlated with the regular growth factor; e.g. it would be wasteful to reallocate storage to 1000 items if it is currently only at 1001...) Something along the lines of this condition, maybe? let delta = requestedCapacity - Swift.max(count, targetCapacity)
let threshold = Swift.max(2 * capacity / 3, 16)
guard delta > threshold else { return }The numbers above are rather arbitrary and if we go this way, finding the right ones will require building a bit of a model. |
||
| if _slowPath(targetCapacity < requestedCapacity) { | ||
| _shrink(targetCapacity: targetCapacity) | ||
| } | ||
| } | ||
|
|
||
| @inlinable | ||
| internal mutating func _shrink(targetCapacity: Int) { | ||
| let minimumCapacity = _reduceCapacity(to: targetCapacity) | ||
| if isUnique() { | ||
| self = self.update { source in | ||
| source.moveElements(minimumCapacity: minimumCapacity) | ||
| } | ||
| } else { | ||
| self = self.read { source in | ||
| source.copyElements(minimumCapacity: minimumCapacity) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,5 +374,40 @@ final class DequeTests: CollectionTestCase { | |
| } | ||
| } | ||
|
|
||
| func test_shrinkCapacityWhenEmpty() { | ||
| withEvery("capacity", in: [0, 1, 2, 5, 10, 50, 100]) { capacity in | ||
| var deque = Deque<Int>(minimumCapacity: capacity) | ||
|
|
||
| XCTAssertGreaterThanOrEqual(deque._capacity, capacity) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Please use the custom |
||
| XCTAssertLessThanOrEqual(deque._requestedCapacity, capacity) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the |
||
|
|
||
| let actual = deque._capacity | ||
| let reducedCapacity = deque._requestedCapacity / 4 | ||
| deque.shrinkCapacity(reducedCapacity) | ||
|
|
||
| print(actual, deque._capacity) | ||
| // Shrinkage isn't guaranteed. | ||
| XCTAssertGreaterThanOrEqual(deque._capacity, reducedCapacity) | ||
| XCTAssertLessThanOrEqual(deque._requestedCapacity, reducedCapacity) | ||
| } | ||
| } | ||
|
|
||
| func test_shrinkCapacityWhenMoreElementsThanTarget() { | ||
| withEvery("count", in: [0, 1, 2, 5, 10, 50, 100]) { count in | ||
| var deque = Deque(repeating: 0, count: count) | ||
| let capacity = deque._capacity | ||
| deque.shrinkCapacity(0) | ||
| XCTAssertEqual(deque._capacity, capacity) | ||
| } | ||
| } | ||
|
|
||
| func test_shrinkCapacityWhenTargetIsLargerThanCurrent() { | ||
| withEvery("capacity", in: [0, 1, 2, 5, 10, 50, 100]) { capacity in | ||
| var deque = Deque<Int>(minimumCapacity: capacity) | ||
| let capacity = deque._capacity | ||
| deque.shrinkCapacity(capacity + 1) | ||
| XCTAssertEqual(deque._capacity, capacity) | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.