Skip to content

Commit

Permalink
Not cause a CoW when calling ByteBuffer.discardReadBytes() while it w…
Browse files Browse the repository at this point in the history
…as fully consumed. (#363)

Motivation:

We should not need to trigger any copy when the ByteBuffer was fully consumed and discardReadBytes() is called.

Modifications:

- If fully consumed just reset reader and writer Index.
- Add unit test

Result:

Less memory copies
  • Loading branch information
normanmaurer authored and Lukasa committed Apr 27, 2018
1 parent cf064a0 commit e5dedbe
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
7 changes: 7 additions & 0 deletions Sources/NIO/ByteBuffer-core.swift
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,13 @@ public struct ByteBuffer {
return false
}

if self._readerIndex == self._writerIndex {
// If the whole buffer was consumed we can just reset the readerIndex and writerIndex to 0 and move on.
self._moveWriterIndex(to: 0)
self._moveReaderIndex(to: 0)
return true
}

if isKnownUniquelyReferenced(&self._storage) {
self._storage.bytes.advanced(by: Int(self._slice.lowerBound))
.copyMemory(from: self._storage.bytes.advanced(by: Int(self._slice.lowerBound + self._readerIndex)),
Expand Down
1 change: 1 addition & 0 deletions Tests/NIOTests/ByteBufferTest+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ extension ByteBufferTest {
("testByteBufferFitsInACoupleOfEnums", testByteBufferFitsInACoupleOfEnums),
("testLargeSliceBegin16MBIsOkayAndDoesNotCopy", testLargeSliceBegin16MBIsOkayAndDoesNotCopy),
("testLargeSliceBeginMoreThan16MBIsOkay", testLargeSliceBeginMoreThan16MBIsOkay),
("testDiscardReadBytesOnConsumedBuffer", testDiscardReadBytesOnConsumedBuffer),
]
}
}
Expand Down
15 changes: 15 additions & 0 deletions Tests/NIOTests/ByteBufferTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,21 @@ class ByteBufferTest: XCTestCase {
XCTAssertEqual(0xdd, slice.getInteger(at: slice.writerIndex - 1, as: UInt8.self))
}

func testDiscardReadBytesOnConsumedBuffer() {
var buffer = self.allocator.buffer(capacity: 8)
buffer.write(integer: 0xaa, as: UInt8.self)
XCTAssertEqual(1, buffer.readableBytes)
XCTAssertEqual(0xaa, buffer.readInteger(as: UInt8.self))
XCTAssertEqual(0, buffer.readableBytes)

var buffer2 = buffer
XCTAssertTrue(buffer.discardReadBytes())
XCTAssertEqual(0, buffer.readerIndex)
XCTAssertEqual(0, buffer.writerIndex)
// As we fully consumed the buffer we should only have adjusted the indices but not triggered a copy as result of CoW sematics.
// So we should still be able to also read the old data.
XCTAssertEqual(0xaa, buffer.getInteger(at: 0, as: UInt8.self))
}
}

private enum AllocationExpectationState: Int {
Expand Down

0 comments on commit e5dedbe

Please sign in to comment.