Add `InlineArray` helpers to `ByteBuffer` #3252

MahdiBM · 2025-05-24T12:45:44Z

Motivation:

Useful for parsing packets, for example ipv4: InlineArray<4, UInt8> and ipv6: InlineArray<16, UInt8>.

Modifications:

For now I've only added a readInlineArray function. I know some other functions are missing, such as writeInlineArray.
I wanted to first open up a discussion and see if these changes are acceptable. I can add those functions too if required, in this PR or other PRs.

Result:

Users can read ByteBuffer into stack-allocated memory, which can be more performant than the other alternatives like Array, or more convenient than reading as a tuple like (UInt8, UInt8, UInt8, UInt8).

Caveats:

Swift 6.2 is required so I've used #if compiler(>=6.2).
Furthermore, InlineArray is marked as available on macOS(9999) since the Swift team have yet to update that mark, although InlineArray is planned for Swift 6.2 per the proposal.

Edit: to be clear things work fine on Linux and that's where I've been using this same readInlineArray function that I've proposed.

MahdiBM · 2025-05-26T19:06:47Z

I'd highly assume a 6.2 snapshot with InlineArray available on macOS would be released before the WWDC. We can wait till then just so the PR is not "weird" in the sense that it works on Linux and not Darwin. We can also merge this and I'll modify the availability checks in another PR as soon as I can confirm the 6.2 macOS snapshots have InlineArray available.

Lukasa · 2025-05-27T10:05:50Z

Sources/NIOCore/ByteBuffer-aux.swift

@@ -58,6 +58,48 @@ extension ByteBuffer {
        return result
    }

+    #if compiler(>=6.2) && !canImport(Darwin)


I don't think we need a canImport guard here: the compiler>=6.2 is sufficient. It does, however, need an availability guard for the presumed next OS releases.

Hmm will it be bound to the next macOS though? It does make sense tbh, that's been the way things have been going (e.g. Mutex in last year's release).
Should I just use "available on macOS 16"?

We can always lower the availability guards without breaking API: we can't raise them. So for now let's assume it's bound to the next OS releases.

Relatedly, we should underscore these names for now to indicate that they aren't necessarily API stable.

As i see it, we have 2 options:

Wait until WWDC so things are more clear. With the rumors of the next macOS being named "macOS 26", using just macOS 16 sounds weird, although it should still have the same effect and require a macOS version high enough. If things somehow go wrong with the macOS availability check, SwiftNIO will fail to build?

Still use macOS 16+friends availability guards?

I'm fine with both. Your call.

As WWDC is just around the corner, I'm happy to wait.

Lukasa · 2025-05-28T14:04:06Z

Sources/NIOCore/ByteBuffer-aux.swift

+        as: InlineArray<count, IntegerType>.Type = InlineArray<count, IntegerType>.self
+    ) -> InlineArray<count, IntegerType>? {
+        let length = MemoryLayout<IntegerType>.size
+        let bytesRequired = length &* count


We can't use unchecked math here today: as it stands with the current Swift implementation, this absolutely can overflow.

Hmm so you think it's possible that someone wants to read that many bytes? I don't think ByteBuffer can fit that anyway? I'll take your word for it but sounds weird to me.
Should I do a multiplyReportingOverflow and throw something on overflow? Should I just precondition no overflow?

Just use regular checked math: length * count. That'll do the checking.

As for whether someone would want to read it: no, not on purpose. But users can easily make mistakes. The goal of a safe language is that those mistakes should not turn into exploitable vulnerabilities, but as written this absolutely would: we'd read off the back of the pointer and we'll all be doomed. If we went back to using getInteger this would be safer, as that would do the bounds checking, but we can have defense in depth.

Done in a054b61

Lukasa · 2025-05-28T14:05:01Z

Sources/NIOCore/ByteBuffer-aux.swift

+            return nil
+        }
+
+        return self.readWithUnsafeReadableBytes {


I don't think this needs unsafe code: this can just delegate to a bunch of getInteger(at:)

That would be of suboptimal performance to use readInteger which delegates to getInteger and moves the readerIndex:
getInteger does a bounds check every time.
getInteger dynamically checks the element type every time.
readInteger moves the reader index only 1 by 1.

What should I do considering these? Should i try to copy getInteger implementation just so we do 1 of each of those operations, or is the current implementation good enough?
I can try to manually benchmark and see which way is more performant too if you think i should.

I guess you said delegate to getInteger, not readInteger, but still we'll have 2 of those 3 problems.

We don't have two of those problems, just one. The dynamic type check only happens if the generic type can't be specialized, but it can. As a practical matter it's never actually executed: it'll either be specialized to false or to true, and then constant-folded away.

As for the bounds check, Swift should be able to observe that the bounds check is redundant with our other checks and elide it. If it fails to do so, that's a bug on Swift.

The SSWG policy on unsafe code is that performance is not a sufficiently good reason to use unsafe code. If safe code doesn't perform well enough, we should improve Swift so that it does. In this particular case, we may well be able to rewrite this on top of Span at some point in the future, but for now we should start with the safe implementation until we know it isn't suitable.

Hmm sounds good, I can get back with "optimizations" when i have proper proof/benchmark results that one is non-insignificantly faster than the other.

I was actually thinking the type check should get specialized too, but i haven't taken deep looks at what the compiler does so wasn't sure if it'll actually get specialized in practice or not. Good to know that it does, most of the time.

The SSWG policy on unsafe code is that performance is not a sufficiently good reason to use unsafe code. If safe code doesn't perform well enough, we should improve Swift so that it does. In this particular case, we may well be able to rewrite this on top of Span at some point in the future, but for now we should start with the safe implementation until we know it isn't suitable.

Span would be optimal. The availability checks should also match since InlineArray is even newer which is nice. Though I don't 100% agree with the "we should improve Swift so that it does". Not that we shouldn't do that, but still, I've heard you mention that SwiftNIO tries to take Swift as what it already is, not what it should be (e.g. when trying to fix stuff that are compiler bugs, not something that SwiftNIO has done wrong) so I'd say we can say the same about this situation even though it's not a show/build-stopper like a compiler bug could be, and if the performance difference is non-insignificant and the safety is verify-able by humans, then we should use that.

Lukasa · 2025-05-28T14:08:38Z

Sources/NIOCore/ByteBuffer-aux.swift

+        endianness: Endianness = .big,
+        as: InlineArray<count, IntegerType>.Type = InlineArray<count, IntegerType>.self
+    ) -> InlineArray<count, IntegerType>? {
+        let length = MemoryLayout<IntegerType>.size


An interesting question here is whether we want to support integer types where stride and size don't match, and what it should imply.

Interesting I agree, but I think the whole SwiftNIO doesn't support that right now so we can worry about it later 😅

I don't think that's true. readMultipleIntegers assumes a packed representation (no padding). We should probably just do that here as well.

Ok thanks for the correction. Will take a look and see how it works out with everything, including using getInteger(at:).

See: 4ab850e
Does this cut it or can it cause problems?

MahdiBM force-pushed the mmbm-inline-array branch 2 times, most recently from f0ded82 to a825a97 Compare May 24, 2025 15:33

Add read InlineArray + tests

eeaaf3e

MahdiBM force-pushed the mmbm-inline-array branch from a825a97 to eeaaf3e Compare May 24, 2025 16:23

Lukasa reviewed May 28, 2025

View reviewed changes

MahdiBM added 5 commits May 30, 2025 22:22

use getInteger(at:)

a054b61

use @_spi(InlineArray) + don't user !Darwin + better comments

7cb4052

Merge branch 'main' into mmbm-inline-array

f8d1dbc

some comments

7470c9f

use stride to account for padding bytes

4ab850e

MahdiBM requested a review from Lukasa May 31, 2025 17:19

Add InlineArray helpers to ByteBuffer #3252

Are you sure you want to change the base?

Add InlineArray helpers to ByteBuffer #3252

Conversation

MahdiBM commented May 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation:

Modifications:

Result:

Caveats:

Uh oh!

MahdiBM commented May 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MahdiBM May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MahdiBM May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MahdiBM May 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Add `InlineArray` helpers to `ByteBuffer` #3252

Add `InlineArray` helpers to `ByteBuffer` #3252

MahdiBM commented May 24, 2025 •

edited

Loading

MahdiBM May 31, 2025 •

edited

Loading

MahdiBM May 29, 2025 •

edited

Loading

MahdiBM May 31, 2025 •

edited

Loading