-
Notifications
You must be signed in to change notification settings - Fork 295
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
Let deque have its capacity shrunk #316
base: main
Are you sure you want to change the base?
Conversation
Deque is often used as a buffer and buffers can be subject to "spiky" sizes; many elements may be buffered before being removed which can result in applications holding on to more memory than desired. Since the capacity of deque is an implementation detail, one way to reduce the size of the deque is asking it to shrink itself. This change adds a `srinkCapacity` method which will reduce the storage capacity of a deque if: - there are fewer elements in the deque than the target capacity - the target capacity is less than the previous requested capacity (the actual allocated capacity may be larger than the requested)
Why do you want separate control of shrinkage vs capacity reservation? Would it make sense to have reserveCapacity shrink if the current buffer is larger than the desired target capacity? |
withEvery("capacity", in: [0, 1, 2, 5, 10, 50, 100]) { capacity in | ||
var deque = Deque<Int>(minimumCapacity: capacity) | ||
|
||
XCTAssertGreaterThanOrEqual(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.
Nit: Please use the custom expect*
assertions from _CollectionsTestSupport -- XCTAssert*
does not know about the context, so its failure messages will not identify the case that failed, making debugging much harder.
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.
Thanks for tackling this! 👍
var deque = Deque<Int>(minimumCapacity: capacity) | ||
|
||
XCTAssertGreaterThanOrEqual(deque._capacity, capacity) | ||
XCTAssertLessThanOrEqual(deque._requestedCapacity, 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.
Can the _requestedCapacity
of the created deque ever be below the capacity
we requested? (I.e., shouldn't this be expecting the two values to be equal?)
@inlinable | ||
@inline(__always) | ||
internal mutating func shrink(targetCapacity: Int) { | ||
if count > targetCapacity { return } |
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.
Hm, this raises a question: for a Deque
with a handful of elements that is backed by a humungous buffer, wouldn't we want to allow the buffer to get shrunk to fit by calling shrink(targetCapacity: count)
(or shrink(targetCapacity: 0)
)?
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.
Description
Deque is often used as a buffer and buffers can be subject to "spiky" sizes; many elements may be buffered before being removed which can result in applications holding on to more memory than desired. Since the capacity of deque is an implementation detail, one way to reduce the size of the deque is asking it to shrink itself.
Related discussion #308
Detailed Design
Documentation
On the new method only.
Yes.
Testing
Performance
I didn't.
Source Impact
API addition only
Checklist