Skip to content
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

Issue-2734 - Clamp buffer to maximum upon large write operation #2745

Merged
merged 18 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dcb0883
Issue-2734 - Clamp buffer to maximum upon large write operation
ali-ahsan-ali Jun 19, 2024
f5e43d0
Issue-2734 - Rename variable name on incorrect assumption
ali-ahsan-ali Jun 19, 2024
e02f3ef
Issue-2734 - Add extra checks to buffer clamp and edit api initialiser
ali-ahsan-ali Jun 24, 2024
01e8cf3
Issue-2734 - Clamp on readable bytes
ali-ahsan-ali Jun 26, 2024
825a6b3
Issue-2743 - Clamp after Flush
ali-ahsan-ali Jul 4, 2024
03f80cf
Issue-2743 - Add documentation for parameter
ali-ahsan-ali Jul 10, 2024
45f6b06
Issue-2734 - Address Feedback and reallocSlice
ali-ahsan-ali Jul 20, 2024
a48b322
Issues-2734 - Fix comment on shrink buffer function
ali-ahsan-ali Jul 27, 2024
8a05b6a
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
weissi Aug 5, 2024
3da1d3a
Issue-2734 - Create static function for padding
ali-ahsan-ali Aug 6, 2024
56e2552
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
ali-ahsan-ali Aug 6, 2024
99c2274
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
Lukasa Aug 9, 2024
cb952f9
Issue-2734 - Address comment about unwritten bytes
ali-ahsan-ali Aug 10, 2024
4c8d94a
Issue-2734 - Address comment about rounding function
ali-ahsan-ali Sep 13, 2024
d5a2751
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
weissi Sep 13, 2024
d9c6e8a
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
weissi Sep 16, 2024
a8b8aec
Merge branch 'main' into Issue-2734-shrink-buffer-after-write
FranzBusch Sep 16, 2024
23ad4a0
Issue-2734 - Update doc
Sep 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions Sources/NIOCore/ByteBuffer-core.swift
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,12 @@ public struct ByteBuffer {
@inline(never)
mutating func _copyStorageAndRebase(capacity: _Capacity, resetIndices: Bool = false) {
// This math has to be very careful, because we already know that in some call paths _readerIndex exceeds 1 << 24, and lots of this math
// is in UInt32 space. It's not hard for us to trip some of these conditions. As a result, I've heavily commented this
// fairly heavily to explain the math.
// is in UInt32 space. It's not hard for us to trip some of these conditions.
// As a result, I've heavily commented this function to explain the math.

// Step 1: If we are resetting the indices, we need to slide the allocation by at least the current value of _readerIndex, so the new
// value of _readerIndex will be 0. Otherwise we can leave them as they are.
// Resetting the indices will also ensure that leading unwritten bytes are not copied.
let indexRebaseAmount = resetIndices ? self._readerIndex : 0

// Step 2: We also want to only copy the bytes within the slice, and move them to index 0. As a result, we have this
Expand Down Expand Up @@ -885,6 +886,30 @@ public struct ByteBuffer {
return true
}

/// The `ByteBuffer` will successfully be shrunk if the requested capacity is less than the current capacity,
/// and the requested capacity is more than the number of readable bytes in the buffer.
ali-ahsan-ali marked this conversation as resolved.
Show resolved Hide resolved
/// If either condition is not true, the buffer will not be shrunk.
///
/// - Parameter desiredCapacity: The desired capacity for the buffers capacity to be shrunken to
/// - Returns: Bool indicating whether the buffer capacity has been shrunk to the desiredCapacity.
@inlinable
@discardableResult public mutating func shrinkBufferCapacity(to desiredCapacity: Int) -> Bool {
let desiredCapacity = ByteBuffer.addPaddingTo(desiredCapacity)
guard desiredCapacity < self.capacity, desiredCapacity > self.readableBytes else {
ali-ahsan-ali marked this conversation as resolved.
Show resolved Hide resolved
return false
}
self._copyStorageAndRebase(capacity: _toCapacity(desiredCapacity), resetIndices: true)
return true
}

/// Returns size of capacity with optimal padding
/// - Parameter initialCapacity: Capacity that needs expansion with padding
/// - Returns: Capacity with calculated padding
@inlinable
static func addPaddingTo(_ initialCapacity: Int) -> Int {
ali-ahsan-ali marked this conversation as resolved.
Show resolved Hide resolved
initialCapacity == 0 ? 0 : initialCapacity.nextPowerOf2()
ali-ahsan-ali marked this conversation as resolved.
Show resolved Hide resolved
}

/// The reader index or the number of bytes previously read from this `ByteBuffer`. `readerIndex` is `0` for a
/// newly allocated `ByteBuffer`.
@inlinable
Expand Down
14 changes: 14 additions & 0 deletions Sources/NIOCore/Codec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -793,9 +793,16 @@ public final class MessageToByteHandler<Encoder: MessageToByteEncoder>: ChannelO
private var state: State = .notInChannelYet
private let encoder: Encoder
private var buffer: ByteBuffer? = nil
private let desiredBufferCapacity: Int?

public init(_ encoder: Encoder, desiredBufferCapacity: Int) {
self.encoder = encoder
self.desiredBufferCapacity = desiredBufferCapacity
}

public init(_ encoder: Encoder) {
self.encoder = encoder
self.desiredBufferCapacity = nil
}
}

Expand Down Expand Up @@ -844,4 +851,11 @@ extension MessageToByteHandler {
context.fireErrorCaught(error)
}
}

public func flush(context: ChannelHandlerContext) {
if let desiredBufferCapacity = self.desiredBufferCapacity {
self.buffer?.shrinkBufferCapacity(to: desiredBufferCapacity)
}
context.flush()
}
}
55 changes: 55 additions & 0 deletions Tests/NIOCoreTests/ByteBufferTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1859,6 +1859,61 @@ class ByteBufferTest: XCTestCase {
XCTAssertEqual(0, buffer2.readableBytes)
}

func testShrinkBufferCapacityWithNoLeadingUnwrittenBytes() {
let desiredCapacity = 1024
var buffer = self.allocator.buffer(capacity: 512)

// For any item, it should not shrink buffer capacity to a value larger than the current buffer capacity
buffer.clear()
buffer.writeString("Any item")
XCTAssertFalse(buffer.shrinkBufferCapacity(to: 2048))
XCTAssertEqual(buffer.capacity, 512)

// If desired capacity are less than or equal to buffer capacity, should not shrink
buffer.clear()
buffer.writeString(String(repeating: "x", count: desiredCapacity))
XCTAssertEqual(buffer.capacity, 1024) // Before
XCTAssertFalse(buffer.shrinkBufferCapacity(to: desiredCapacity))
XCTAssertEqual(buffer.capacity, 1024) // After

// If desiredCapacity is less than readable bytes, do not shrink
buffer.clear()
buffer.writeString(String(repeating: "x", count: desiredCapacity + 1))
XCTAssertEqual(buffer.capacity, 2048)
XCTAssertFalse(buffer.shrinkBufferCapacity(to: desiredCapacity))
XCTAssertEqual(buffer.capacity, 2048)

// If desired capacity is greater than the readable bytes and less than buffer capacity, should shrink to desiredCapacity.nextPowerOf2Clamp
// Where desiredCapacity.nextPowerOf2Clamp == desiredCapacity as desiredCapacity should already be a power of 2 value
buffer.clear()
buffer.writeString(String(repeating: "x", count: desiredCapacity - 1))
XCTAssertEqual(buffer.capacity, 2048)
XCTAssertTrue(buffer.shrinkBufferCapacity(to: desiredCapacity))
XCTAssertEqual(buffer.capacity, 1024)
}

func testShrinkBufferCapacityWithLeadingUnwrittenBytes() {
var buffer = self.allocator.buffer(capacity: 16384)
buffer.moveWriterIndex(to: 16000)
buffer.moveReaderIndex(to: 16000)
buffer.writeString("WW")
buffer.shrinkBufferCapacity(to: 4)
XCTAssertEqual("WW", String(buffer: buffer))

// If readable bytes is exactly the same as buffer capacity shrunken to
buffer = self.allocator.buffer(capacity: 16)
buffer.moveWriterIndex(to: 8)
buffer.moveReaderIndex(to: 8)
buffer.writeString("WWWWWWWW") // 8 bytes written
buffer.shrinkBufferCapacity(to: 4) // Invisible padding makes this 8 bytes
XCTAssertEqual("WWWWWWWW", String(buffer: buffer)) // All 8 bytes are returned!
ali-ahsan-ali marked this conversation as resolved.
Show resolved Hide resolved
}

func testExpansionOfCapacityWithPadding() throws {
XCTAssertEqual(ByteBuffer.addPaddingTo(12), 16)
XCTAssertEqual(ByteBuffer.addPaddingTo(0), 0)
}

func testDumpBytesFormat() throws {
self.buf.clear()
for f in UInt8.min...UInt8.max {
Expand Down