Add some spans to ByteBuffer #3371
Conversation
Motivation: Users would like to be able to access the underlying memory of a ByteBuffer, as evidenced by the plethora of `withUnsafe*` methods that ByteBuffer has. As Swift 6.2 has introduced some initial APIs for safe memory access to underlying storage, we should offer similar APIs on ByteBuffer to enable users to get safer access to that storage. For now, the obvious APIs to be able to supplement are: - withUnsafeReadableBytes - withUnsafeMutableReadableBytes - writeWithUnsafeMutableWritableBytes We can also offer some new APIs to allow initializing a buffer directly from an OutputSpan. Note that we can only do this because the Language Steering Group has pinky promised that they will not break the "Lifetimes" experimental feature: see https://forums.swift.org/t/experimental-support-for-lifetime-dependencies-in-swift-6-2-and-beyond/78638 for more details. We are taking them at their word, and so we are enabling that feature. Modifications: Many new methods and tests. Result: Safer access.
Sources/NIOCore/ByteBuffer-aux.swift
Outdated
| @available(macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2, visionOS 1.0, *) | ||
| public init( | ||
| initialCapacity capacity: Int, | ||
| initializingWith initializer: (inout OutputRawSpan) throws -> Void |
There was a problem hiding this comment.
super nit, label tuple parameters per https://www.swift.org/documentation/api-design-guidelines/#special-instructions
| initializingWith initializer: (inout OutputRawSpan) throws -> Void | |
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void |
Sources/NIOCore/ByteBuffer-aux.swift
Outdated
| @available(macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2, visionOS 1.0, *) | ||
| public func buffer( | ||
| capacity: Int, | ||
| initializingWith initializer: (inout OutputRawSpan) throws -> Void |
There was a problem hiding this comment.
| initializingWith initializer: (inout OutputRawSpan) throws -> Void | |
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void |
| @available(macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2, visionOS 1.0, *) | ||
| public mutating func write( | ||
| minimumWritableBytes: Int, | ||
| initializingWith initializer: (inout OutputRawSpan) throws -> Void |
There was a problem hiding this comment.
| initializingWith initializer: (inout OutputRawSpan) throws -> Void | |
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void |
| /// Enables high-performance low-level appending into the writable section of this buffer. | ||
| @inlinable | ||
| @available(macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2, visionOS 1.0, *) | ||
| public mutating func write( |
There was a problem hiding this comment.
Should this be something like writeWithOutputRawSpan as an analogue to writeWithUnsafeMutableBytes?
| } | ||
| } | ||
|
|
||
| /// Enables high-performance low-level appending into the writable section of this buffer. |
There was a problem hiding this comment.
Should we document that the writer index is moved?
| public init( | ||
| initialCapacity capacity: Int, | ||
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void | ||
| ) rethrows { |
There was a problem hiding this comment.
Instead of using rethrows can we use typed-throws here which composes better. Similar to the InlineArray init
There was a problem hiding this comment.
So the problem with that is that to do it I have to introduce a typed-throws overload of writeWithUnsafeMutableBytes, which will have a somewhat challenging type signature. I'm not 100% sure it's worth doing at this time.
Unless we think it is API compatible to move from something being untyped throws to typed throws, which I'm not convinced it is.
There was a problem hiding this comment.
I just checked what the stdlib folks did and it looks like they migrated methods that were previously using rethrows to use typed-throws e.g. withUnsafeBufferPointer to withUnsafeBufferPointer. They only kept the ABI entry for the rethrows method by using @usableFromInline. If this worked for the stdlib then it should to work for us as well.
| public func buffer( | ||
| capacity: Int, | ||
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void | ||
| ) rethrows -> ByteBuffer { |
| /// Provides safe high-performance read-only access to the readable bytes of this buffer. | ||
| @inlinable | ||
| @available(macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2, visionOS 1.0, *) | ||
| public var readableBytesSpan: RawSpan { |
There was a problem hiding this comment.
It's unfortunate that we can' follow the stdlib naming conventions here which would name this readableBytes since this already exists on ByteBuffer
| public mutating func writeWithOutputRawSpan( | ||
| minimumWritableBytes: Int, | ||
| initializingWith initializer: (_ span: inout OutputRawSpan) throws -> Void | ||
| ) rethrows { |
There was a problem hiding this comment.
Same here regarding rethrows
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| import XCTest |
There was a problem hiding this comment.
Just a suggestion but since this is already 6.2+ we may wanna consider using Testing here to avoid having to migrate the tests in the future.
4 participants
Motivation:
Users would like to be able to access the underlying memory of a ByteBuffer, as evidenced by the plethora of
withUnsafe*methods that ByteBuffer has. As Swift 6.2 has introduced some initial APIs for safe memory access to underlying storage, we should offer similar APIs on ByteBuffer to enable users to get safer access to that storage.For now, the obvious APIs to be able to supplement are:
We can also offer some new APIs to allow initializing a buffer directly from an OutputSpan.
Note that we can only do this because the Language Steering Group has pinky promised that they will not break the "Lifetimes" experimental feature: see
https://forums.swift.org/t/experimental-support-for-lifetime-dependencies-in-swift-6-2-and-beyond/78638 for more details. We are taking them at their word, and so we are enabling that feature.
Modifications:
Many new methods and tests.
Result:
Safer access.