Don't reserveCapacity in append(contentsOf:), it breaks API guarantees of asymptotic complexity #65927
Conversation
…ion.append(contentsOf:) before falling back to a slow element-by-element loop. Fixes rdar://109059874
…We don't want that, so omit it
…s of asymptotic complexity
|
@swift-ci please benchmark |
|
@swift-ci please test |
There was a problem hiding this comment.
It would probably be a good idea to add a test that ensures that none of the default RangeReplaceableCollection mutation implementations end up calling reserveCapacity, to prevent well-meaning changes that would reintroduce these from landing without breaking a test.
We may have a minimal RRC type in the stdlib already, but if not, we could add a dummy one directly in the test. (It could either count invocations like the existing minimal types, or it could just fatalError in reserveCapacity. 🤔)
| let approximateCapacity = self.count + newElements.underestimatedCount | ||
| self.reserveCapacity(approximateCapacity) |
There was a problem hiding this comment.
It may be a good idea to add an explicit comment about this gotcha, to help prevent this from happening again.
| let approximateCapacity = self.count + newElements.underestimatedCount | |
| self.reserveCapacity(approximateCapacity) | |
| // Note: This used to call `reserveCapacity` here, but that can dramatically | |
| // slow down clients that repeatedly call `append(contentsOf:)`. Do not | |
| // re-add that. (The usual implementation of `reserveCapacity` for | |
| // contiguous collections disables exponential growth for the backing store, | |
| // ruining the amortized O(*m*) complexity that we promise above.) | |
|
@swift-ci please test |
…s of asymptotic complexity (swiftlang#65927) Don't reserveCapacity in append(contentsOf:), it breaks API guarantees of asymptotic complexity. Fixes rdar://109577273
…s of asymptotic complexity (swiftlang#65927) Don't reserveCapacity in append(contentsOf:), it breaks API guarantees of asymptotic complexity. Fixes rdar://109577273
Fixes rdar://109577273