Add Message binary decoding support from ContiguousBytes. #914
Conversation
ContiguousBytes should allow other libraries that have their own "buffer" types to directly decode things without having to copy the payloads into a Data first. - Add a new init. - Add a new merge. - Redirect the Data interfaces thru the ContiguousBytes interfaces.
thomasvl
force-pushed
the
contiguousBytes
branch
from
2c85443 to
b4f3e0a
Compare
|
Sorry, I was on holiday when this patch was posted. Can I suggest that rather than taking a This advantage particularly accrues to data types whose implementation of import Foundation
@usableFromInline
func doStuff(_ pointer: UnsafeRawBufferPointer) throws { }
public func takesExistential(contiguousBytes bytes: ContiguousBytes) throws {
try bytes.withUnsafeBytes(doStuff)
}
@inlinable
public func takesGeneric<Bytes: ContiguousBytes>(contiguousBytes bytes: Bytes) throws {
try bytes.withUnsafeBytes(doStuff)
}In a different module (necessary to simulate the effects of the cross-module optimisation properties of this code) we can import NIO and try to pass a import lib
import NIO
import NIOFoundationCompat
@inline(never)
func testWithExistential(buffer: ByteBuffer) throws {
try takesExistential(contiguousBytes: buffer.readableBytesView)
}
@inline(never)
func testWithGeneric(buffer: ByteBuffer) throws {
try takesGeneric(contiguousBytes: buffer.readableBytesView)
}
var buffer = ByteBufferAllocator().buffer(capacity: 1024)
try! testWithExistential(buffer: buffer)
try! testWithGeneric(buffer: buffer)The difference in output here is profound. The existential-taking code generates two quite large functions. The first is This code is in 3 parts. The first block creates the The third block is interesting, as it does two things. Firstly, it heap-allocates. It does this because the existential being passed to Here we do a partial unwrap of the existential and then jump through the The effect here is that we have the following costs: one allocation, an unwrap of an existential, an indirect call via the PWT into NIO, followed by another indirect call in the non-specialised NIO implementation of By comparison, here is the code generated by This code is a bit larger but has a number of better properties. In particular, it does not heap allocate, and never makes an indirect call: all calls are directly to the relevant function, including directly into In fact, the only problem with this code is that the codegen is kinda lousy due to implementation mistakes in NIO: we missed a bunch of inlinables that I'll be adding sometime soon. In general, having this inlinable generic thunk greatly reduces the costs of obtaining pointers to the backing storage of structures, and I'd strongly recommend doing it. |
|
Do we want to just worry about inline on this or should we also start to go after #763 (and deal with what it might bean for breaking changes). |
|
The generic part is likely easily doable, but
So the question seems to quickly slide into expanding the library interface with more things |
You can if you Mark it |
Yea, that doesn't really work out, everything to support the |
|
No they don't, you just need to refactor slightly. You have this: public mutating func merge(
contiguousBytes bytes: ContiguousBytes,
extensions: ExtensionMap? = nil,
partial: Bool = false,
options: BinaryDecodingOptions = BinaryDecodingOptions()
) throws {
try bytes.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
if let baseAddress = body.baseAddress, body.count > 0 {
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
var decoder = BinaryDecoder(forReadingFrom: pointer,
count: body.count,
options: options,
extensions: extensions)
try decoder.decodeFullMessage(message: &self)
}
}
if !partial && !isInitialized {
throw BinaryDecodingError.missingRequiredFields
}
}You can write this instead: @inlinable
public mutating func merge<Bytes: ContiguousBytes>(
contiguousBytes bytes: Bytes,
extensions: ExtensionMap? = nil,
partial: Bool = false,
options: BinaryDecodingOptions = BinaryDecodingOptions()
) throws {
try bytes.withUnsafeBytes { (body: UnsafeRawBufferPointer) in
try _merge(rawBytes: body, extensions: extensions, partial: partial, options: options)
}
}
@usableFromInline
internal mutating func _merge(
rawBytes body: UnsafeRawBufferPointer,
extensions: ExtensionMap?,
partial: Bool,
options: BinaryDecodingOptions
) throws {
if let baseAddress = body.baseAddress, body.count > 0 {
let pointer = baseAddress.assumingMemoryBound(to: UInt8.self)
var decoder = BinaryDecoder(forReadingFrom: pointer,
count: body.count,
options: options,
extensions: extensions)
try decoder.decodeFullMessage(message: &self)
}
if !partial && !isInitialized {
throw BinaryDecodingError.missingRequiredFields
}
}This requires only one point of |
|
That's the other option I was suggesting, I just wasn't sure if that meant we were getting the same wins since we're only moving a small amount to be reachable now. |
|
Yeah, this gives nearly 100% of the win. As a rough heuristic, if the function is not generic, the win from making it |
|
@Lukasa, do you have any links to good resources where I can read more about the finer performance differences between things like existentials vs. generics? I get bits and pieces of info from discussions like this and from some of the compiler documentation, but it's not an area I've ever needed to personally focus too much on, so I have a lot of gaps in my knowledge and I'd love to know if someone has done a comprehensive write-up anywhere. |
|
I am not aware of a good writeup. It's definitely something worth having written down. The biggest problem is actually that the story changes a lot: for example, Swift recently (i.e. in the last two or so Swift versions) started stack allocating existentials where possible which somewhat changes their performance profile. Part of the issue as well is that constructing small benchmarks is quite hard because you need to place a module boundary in the way. Within a module, the compiler will usually be able to break down the abstraction and perform the appropriate specialisation work. Only across boundaries do we see the difference. In general, though, my first order theory is that wherever possible it is better to use generics than existentials, because generics can be specialised but existentials cannot. This rule is not 100% factually accurate, but it's a good starting point: preserving type information gives the compiler more options than throwing it away. Naturally this doesn't work in all cases: generics are tricky for heterogeneous containers, for example. But it's a useful starting point. |
ContiguousBytes should allow other libraries that have their own "buffer"
types to directly decode things without having to copy the payloads into
a Data first.