From e9aa6926ef8aa249ee8b26d0c9dc7ff45dd19314 Mon Sep 17 00:00:00 2001 From: Esteban Torres Date: Thu, 12 Jul 2018 08:58:42 +0200 Subject: [PATCH] Make CircularBuffer conform to more protocols [Fixes #350] (#466) Motivation: During the `PR` apple/swift-nio#347 there was a discussion around which protocols this autoexpanding buffer should conform; the following 3 being the winners: 1. BidirectionalCollection 2. RandomAccessCollection 3. RangeReplaceableCollection Modifications: Added conformance to the 3 protocolos and provided implementaiton for 4 methods: 1. replaceSubrange(:with:) 2. index(before:) 3. removeSubrange(:) 4. remove(at:) 5. removeLast(:) Result: With the conformance of this 3 protocols and the implementation of this 5 functions we get dropLast(), dropLast(:), removeFirst(), removeLast() & popLast() for free. --- Sources/NIO/CircularBuffer.swift | 203 ++++++++++++------ .../NIOTests/CircularBufferTests+XCTest.swift | 14 ++ Tests/NIOTests/CircularBufferTests.swift | 182 +++++++++++++++- 3 files changed, 335 insertions(+), 64 deletions(-) diff --git a/Sources/NIO/CircularBuffer.swift b/Sources/NIO/CircularBuffer.swift index 77c609e8f9..03ea433a59 100644 --- a/Sources/NIO/CircularBuffer.swift +++ b/Sources/NIO/CircularBuffer.swift @@ -52,6 +52,11 @@ public struct CircularBuffer: CustomStringConvertible, AppendableCollection { assert(self.buffer.count == capacity) } + /// Allocates an empty buffer. + public init() { + self.init(initialRingCapacity: 16) + } + /// Append an element to the end of the ring buffer. /// /// Amortized *O(1)* @@ -97,60 +102,6 @@ public struct CircularBuffer: CustomStringConvertible, AppendableCollection { self.buffer = newBacking } - /// Remove the front element of the ring buffer. - /// - /// *O(1)* - public mutating func removeFirst() -> E { - guard let value = self.buffer[self.headIdx] else { - preconditionFailure("CircularBuffer is empty") - } - self.buffer[self.headIdx] = nil - self.headIdx = (self.headIdx + 1) & self.mask - - return value - } - - /// Return the first element of the ring. - /// - /// *O(1)* - public var first: E? { - if self.isEmpty { - return nil - } else { - return self.buffer[self.headIdx] - } - } - - /// Remove the last element of the ring buffer. - /// - /// *O(1)* - public mutating func removeLast() -> E { - let idx = (self.tailIdx - 1) & self.mask - guard let value = self.buffer[idx] else { - preconditionFailure("CircularBuffer is empty") - } - self.buffer[idx] = nil - self.tailIdx = idx - - return value - } - - /// Return the last element of the ring. - /// - /// *O(1)* - public var last: E? { - if self.isEmpty { - return nil - } else { - return self.buffer[(self.tailIdx - 1) & self.mask] - } - } - - private func bufferIndex(ofIndex index: Int) -> Int { - precondition(index < self.count, "index out of range") - return (self.headIdx + index) & self.mask - } - // MARK: Collection implementation /// Return element `index` of the ring. /// @@ -164,15 +115,6 @@ public struct CircularBuffer: CustomStringConvertible, AppendableCollection { } } - /// Slice out a range of the ring. - /// - /// *O(1)* - public subscript(bounds: Range) -> Slice> { - get { - return Slice(base: self, bounds: bounds) - } - } - /// Return all valid indices of the ring. public var indices: RangeType { return 0..: CustomStringConvertible, AppendableCollection { return nextIndex } + /// Returns the index before `index`. + public func index(before: Int) -> Int { + precondition(before > 0) + return before - 1 + } + /// Removes all members from the circular buffer whist keeping the capacity. public mutating func removeAll(keepingCapacity: Bool = false) { self.headIdx = 0 @@ -234,3 +182,132 @@ public struct CircularBuffer: CustomStringConvertible, AppendableCollection { return desc } } + +// MARK: - BidirectionalCollection, RandomAccessCollection, RangeReplaceableCollection +extension CircularBuffer: BidirectionalCollection, RandomAccessCollection, RangeReplaceableCollection { + /// Replaces the specified subrange of elements with the given collection. + /// + /// - Parameter subrange: + /// The subrange of the collection to replace. The bounds of the range must be valid indices of the collection. + /// + /// - Parameter newElements: + /// The new elements to add to the collection. + /// + /// *O(n)* where _n_ is the length of the new elements collection if the subrange equals to _n_ + /// + /// *O(m)* where _m_ is the combined length of the collection and _newElements_ + public mutating func replaceSubrange(_ subrange: Range, with newElements: C) where C : Collection, E == C.Element { + precondition(subrange.lowerBound >= self.startIndex && subrange.upperBound <= self.endIndex, "Subrange out of bounds") + + if subrange.count == newElements.count { + // Can't just zip(subrange, newElements) because the compiler complains about: + // «argument type 'Range' does not conform to expected type 'Sequence'» + // with Swift version 4.1.2 (swiftlang-902.0.54 clang-902.0.39.2) + for (index, element) in zip(subrange.lowerBound.. = [] + let capacityDelta = (Int(newElements.count) - subrange.count) + let newCapacity = Int(UInt32(self.buffer.count + capacityDelta).nextPowerOf2()) + newBuffer.reserveCapacity(newCapacity) + + // This mapping is required due to an inconsistent ability to append sequences of non-optional + // to optional sequences. + // https://bugs.swift.org/browse/SR-7921 + newBuffer.append(contentsOf: self[0.. 0 { + newBuffer.append(contentsOf: repeatElement(nil, count: repetitionCount)) + } + self.headIdx = 0 + self.buffer = newBuffer + } + } + + /// Removes the elements in the specified subrange from the circular buffer. + /// + /// - Parameter bounds: The range of the circular buffer to be removed. The bounds of the range must be valid indices of the collection. + public mutating func removeSubrange(_ bounds: Range) { + precondition(bounds.upperBound >= self.startIndex && bounds.upperBound <= self.endIndex, "Invalid bounds.") + switch bounds.count { + case 1: + _ = remove(at: bounds.lowerBound) + case self.count: + self = .init(initialRingCapacity: self.buffer.count) + default: + replaceSubrange(bounds, with: []) + } + } + + /// Removes the given number of elements from the end of the collection. + /// + /// - Parameter n: The number of elements to remove from the tail of the buffer. + public mutating func removeLast(_ n: Int) { + precondition(n <= self.count, "Number of elements to drop bigger than the amount of elements in the buffer.") + var idx = self.tailIdx + for _ in 0 ..< n { + self.buffer[idx] = nil + idx = self.bufferIndex(before: idx) + } + self.tailIdx = (self.tailIdx - n) & self.mask + } + + /// Removes & returns the item at `position` from the buffer + /// + /// - Parameter position: The index of the item to be removed from the buffer. + /// + /// *O(1)* if the position is `headIdx` or `tailIdx`. + /// otherwise + /// *O(n)* where *n* is the number of elements between `position` and `tailIdx`. + public mutating func remove(at position: Int) -> E { + precondition(self.indices.contains(position), "Position out of bounds.") + var bufferIndex = self.bufferIndex(ofIndex: position) + let element = self.buffer[bufferIndex]! + + switch bufferIndex { + case self.headIdx: + self.headIdx = self.bufferIndex(after: self.headIdx) + self.buffer[bufferIndex] = nil + case self.tailIdx - 1: + self.tailIdx = self.bufferIndex(before: self.tailIdx) + self.buffer[bufferIndex] = nil + default: + var nextIndex = self.bufferIndex(after: bufferIndex) + while nextIndex != self.tailIdx { + self.buffer[bufferIndex] = self.buffer[nextIndex] + bufferIndex = nextIndex + nextIndex = self.bufferIndex(after: bufferIndex) + } + self.buffer[nextIndex] = nil + self.tailIdx = self.bufferIndex(before: self.tailIdx) + } + + return element + } +} + +// MARK: - Private functions + +private extension CircularBuffer { + func bufferIndex(ofIndex index: Int) -> Int { + precondition(index < self.count, "index out of range") + return (self.headIdx + index) & self.mask + } + + /// Returns the internal buffer next index after `index`. + func bufferIndex(after: Int) -> Int { + return (after + 1) & self.mask + } + + /// Returns the internal buffer index before `index`. + func bufferIndex(before: Int) -> Int { + return (before - 1) & self.mask + } +} diff --git a/Tests/NIOTests/CircularBufferTests+XCTest.swift b/Tests/NIOTests/CircularBufferTests+XCTest.swift index f973b8d524..828febe35a 100644 --- a/Tests/NIOTests/CircularBufferTests+XCTest.swift +++ b/Tests/NIOTests/CircularBufferTests+XCTest.swift @@ -29,8 +29,18 @@ extension CircularBufferTests { ("testTrivial", testTrivial), ("testAddRemoveInALoop", testAddRemoveInALoop), ("testAddAllRemoveAll", testAddAllRemoveAll), + ("testRemoveAt", testRemoveAt), + ("testRemoveAtLastPosition", testRemoveAtLastPosition), + ("testRemoveAtTailIdx0", testRemoveAtTailIdx0), + ("testRemoveAtFirstPosition", testRemoveAtFirstPosition), ("testHarderExpansion", testHarderExpansion), ("testCollection", testCollection), + ("testReplaceSubrange5ElementsWith1", testReplaceSubrange5ElementsWith1), + ("testReplaceSubrangeAllElementsWithFewerElements", testReplaceSubrangeAllElementsWithFewerElements), + ("testReplaceSubrangeEmptyRange", testReplaceSubrangeEmptyRange), + ("testReplaceSubrangeWithSubrangeLargerThanTargetRange", testReplaceSubrangeWithSubrangeLargerThanTargetRange), + ("testReplaceSubrangeSameSize", testReplaceSubrangeSameSize), + ("testReplaceSubrangeReplaceBufferWithEmptyArray", testReplaceSubrangeReplaceBufferWithEmptyArray), ("testWeCanDistinguishBetweenEmptyAndFull", testWeCanDistinguishBetweenEmptyAndFull), ("testExpandZeroBasedRingWorks", testExpandZeroBasedRingWorks), ("testExpandNonZeroBasedRingWorks", testExpandNonZeroBasedRingWorks), @@ -40,10 +50,14 @@ extension CircularBufferTests { ("testCount", testCount), ("testFirst", testFirst), ("testLast", testLast), + ("testRemoveLast", testRemoveLast), + ("testRemoveLastCountElements", testRemoveLastCountElements), + ("testRemoveLastElements", testRemoveLastElements), ("testOperateOnBothSides", testOperateOnBothSides), ("testPrependExpandBuffer", testPrependExpandBuffer), ("testRemoveAllKeepingCapacity", testRemoveAllKeepingCapacity), ("testRemoveAllNotKeepingCapacity", testRemoveAllNotKeepingCapacity), + ("testBufferManaged", testBufferManaged), ] } } diff --git a/Tests/NIOTests/CircularBufferTests.swift b/Tests/NIOTests/CircularBufferTests.swift index 8e86bd233a..7baa04a8af 100644 --- a/Tests/NIOTests/CircularBufferTests.swift +++ b/Tests/NIOTests/CircularBufferTests.swift @@ -51,6 +51,52 @@ class CircularBufferTests: XCTestCase { XCTAssertTrue(ring.isEmpty) } + func testRemoveAt() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<7 { + ring.prepend(idx) + } + + XCTAssertEqual(7, ring.count) + _ = ring.remove(at: 1) + XCTAssertEqual(6, ring.count) + XCTAssertEqual(0, ring.last) + } + + func testRemoveAtLastPosition() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<7 { + ring.prepend(idx) + } + + let last = ring.remove(at: ring.endIndex - 1) + XCTAssertEqual(0, last) + XCTAssertEqual(1, ring.last) + } + + func testRemoveAtTailIdx0() { + var ring = CircularBuffer(initialRingCapacity: 4) + ring.prepend(99) + ring.prepend(98) + XCTAssertEqual(2, ring.count) + XCTAssertEqual(99, ring.remove(at: ring.endIndex - 1)) + XCTAssertFalse(ring.isEmpty) + XCTAssertEqual(1, ring.count) + XCTAssertEqual(98, ring.last) + XCTAssertEqual(98, ring.first) + } + + func testRemoveAtFirstPosition() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<7 { + ring.prepend(idx) + } + + let first = ring.remove(at: 0) + XCTAssertEqual(6, first) + XCTAssertEqual(5, ring.first) + } + func testHarderExpansion() { var ring = CircularBuffer(initialRingCapacity: 3) XCTAssertEqual(ring.indices, 0..<0) @@ -125,12 +171,100 @@ class CircularBufferTests: XCTestCase { XCTAssertEqual(ring.endIndex, 5) XCTAssertEqual(ring.index(after: 1), 2) - + XCTAssertEqual(ring.index(before: 3), 2) + let actualValues = [Int](ring) let expectedValues = [0, 1, 2, 3, 4] XCTAssertEqual(expectedValues, actualValues) } + func testReplaceSubrange5ElementsWith1() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<50 { + ring.prepend(idx) + } + XCTAssertEqual(50, ring.count) + ring.replaceSubrange(20..<25, with: [99]) + + XCTAssertEqual(ring.count, 46) + XCTAssertEqual(ring[19], 30) + XCTAssertEqual(ring[20], 99) + XCTAssertEqual(ring[21], 24) + } + + func testReplaceSubrangeAllElementsWithFewerElements() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<50 { + ring.prepend(idx) + } + XCTAssertEqual(50, ring.count) + + ring.replaceSubrange(ring.startIndex..(initialRingCapacity: 4) + for idx in 0..<50 { + ring.prepend(idx) + } + XCTAssertEqual(50, ring.count) + + ring.replaceSubrange(0..<0, with: []) + XCTAssertEqual(50, ring.count) + } + + func testReplaceSubrangeWithSubrangeLargerThanTargetRange() { + var ring = CircularBuffer(initialRingCapacity: 4) + for idx in 0..<5 { + ring.prepend(idx) + } + XCTAssertEqual(5, ring.count) + + ring.replaceSubrange(ring.startIndex..(initialRingCapacity: 4) + for idx in 0..<5 { + ring.prepend(idx) + } + XCTAssertEqual(5, ring.count) + XCTAssertEqual(4, ring.first) + XCTAssertEqual(0, ring.last) + + var replacement = [Int]() + for idx in 0..<5 { + replacement.append(idx) + } + XCTAssertEqual(5, replacement.count) + XCTAssertEqual(0, replacement.first) + XCTAssertEqual(4, replacement.last) + + ring.replaceSubrange(ring.startIndex..(initialRingCapacity: 4) + for idx in 0..<5 { + ring.prepend(idx) + } + XCTAssertEqual(5, ring.count) + XCTAssertEqual(4, ring.first) + XCTAssertEqual(0, ring.last) + + ring.replaceSubrange(ring.startIndex..(initialRingCapacity: 4) XCTAssertTrue(ring.isEmpty) @@ -268,6 +402,41 @@ class CircularBufferTests: XCTestCase { XCTAssertTrue(ring.isEmpty) } + func testRemoveLast() { + var ring = CircularBuffer(initialRingCapacity: 3) + XCTAssertNil(ring.last) + ring.append(1) + ring.prepend(0) + XCTAssertEqual(1, ring.last) + XCTAssertEqual(1, ring.removeLast()) + XCTAssertEqual(0, ring.last) + } + + func testRemoveLastCountElements() { + var ring = CircularBuffer(initialRingCapacity: 3) + XCTAssertNil(ring.last) + ring.append(1) + ring.prepend(0) + XCTAssertEqual(1, ring.last) + ring.removeLast(2) + XCTAssertTrue(ring.isEmpty) + } + + func testRemoveLastElements() { + var ring = CircularBuffer(initialRingCapacity: 10) + XCTAssertNil(ring.last) + for i in 0 ..< 20 { + ring.prepend(i) + } + XCTAssertEqual(20, ring.count) + XCTAssertEqual(19, ring.first) + XCTAssertEqual(0, ring.last) + ring.removeLast(10) + XCTAssertEqual(10, ring.count) + XCTAssertEqual(19, ring.first) + XCTAssertEqual(10, ring.last) + } + func testOperateOnBothSides() { var ring = CircularBuffer(initialRingCapacity: 3) XCTAssertNil(ring.last) @@ -335,4 +504,15 @@ class CircularBufferTests: XCTestCase { XCTAssertEqual(ring.capacity, 1) XCTAssertEqual(ring.count, 0) } + + func testBufferManaged() { + var ring = CircularBuffer(initialRingCapacity: 2) + ring.append(1) + XCTAssertEqual(ring.capacity, 2) + + // Now we want to replace the last subrange with two elements. This should + // force an increase in size. + ring.replaceSubrange(0..<1, with: [3, 4]) + XCTAssertEqual(ring.capacity, 4) + } }