[swift] support for unaligned buffers

[mr-swifter]

Usually when reading data from network or from binary file there is no guarantee that data will be placed in memory with respect to alignment. So, trying to use such memory buffers as is will lead to precondition.

In swift 5.7 there is a new loadUnaligned function is available, so plain data types can be accessed without respect to alignment. So, seems that means by using loadUnaligned we can create flatbuffer from any buffer and it will work normally.

So, I suggest to start using loadUnaligned instead of load. storeBytes works without changing function signature. Do i miss anything?

[mustiikhalil]

So flatbuffers usually do align the buffers according to what they will be used for.
For example here, where we Pre-align the buffer to write the data properly. So in terms of flatbuffers being safe to be written in swift they are.

  @inline(__always)
  @discardableResult
  mutating public func push<T: Scalar>(element: T) -> UOffset {
    let size = MemoryLayout<T>.size
    preAlign(
      len: size,
      alignment: size)
    _bb.push(value: element, len: size)
    return _bb.size
  }

Regarding the reading, we should always verify the buffer especially if they are coming from a network. which uses the following function:

public func getCheckedRoot<T: FlatBufferObject & Verifiable>(
  byteBuffer: inout ByteBuffer,
  fileId: String? = nil,
  options: VerifierOptions = .init()) throws -> T
{
  var verifier = try Verifier(buffer: &byteBuffer, options: options)
  if let fileId = fileId {
    try verifier.verify(id: fileId)
  }
  try ForwardOffset<T>.verify(&verifier, at: 0, of: T.self)
  return T.init(
    byteBuffer,
    o: Int32(byteBuffer.read(def: UOffset.self, position: byteBuffer.reader)) +
      Int32(byteBuffer.reader))
}

This will always check for alignment, and content according to what the code is supposed to actual read.
By default we always check for alignment, but if you want to skip that, you can always re-write the function yourself.

[mustiikhalil]

@dbaileychess please do correct me if i'm wrong regarding writing into a flatbuffer and that the values are always aligned

[mr-swifter]

By default we always check for alignment, but if you want to skip that, you can always re-write the function yourself.

I'm not sure I understand what do you mean by "re-write the function yourself". It's flatbuffers API function, isn't it?

Maybe I should be a bit more precise in use case I want to fix. For example, I have something like:

func handleMessage(message: UnsafeMutableRawPointer, capacity: Int) { 
// handle function called with pointer to some buffer allocated and read from network somewhere. 
// I don't what to copy it, so I want to create flat buffer message on top of it
let byteBuffer = ByteBuffer(assumingMemoryBound: message, capacity: capacity)
// Now byte buffer keep unowned reference to memory buffer, let's try to create flatbuffer
let myMessage = MyMessage.getRootAsMyMessage(bb: byteBuffer)
// And I get precondition here! 

I don't see much difference vs getCheckedRoot since any read, for example, byteBuffer.read(def: UOffset.self, position: byteBuffer.reader) can cause the same problem since it ends up with loading UOffset from memory.

[mustiikhalil]

@mr-swifter

I'm not sure I understand what do you mean by "re-write the function yourself". It's flatbuffers API function, isn't it?

Yes it is, but it's written that way so that users can copy and paste the code and modify it as they please. for example, the verifier struct can also be created manually and from there we can set the alignment checking to false.

Is the message aka UnsafeMutableRawPointer a valid single object flatbuffer?

Because I do understand, the fact that there is an issue reading the buffer, but if the buffer wasn't aligned properly first then the issue might be in creating the buffer, or the buffer isn't complete. and honestly i would rather have the precondition fail safe rather than just allowing unaligned reading from memory.

But let's see what @dbaileychess would say regarding the alignment.
If you have example of the buffer that would be also great!

[mustiikhalil]

I don't see much difference vs getCheckedRoot since any read, for example, byteBuffer.read(def: UOffset.self, position: byteBuffer.reader) can cause the same problem since it ends up with loading UOffset from memory.

Yes it does read the Uoffset but after it validates if its a proper Uoffset that will not break the app. If you look at this you will see we are validating the UOffset first, and validating that the UOffset is within the buffer

  try ForwardOffset<T>.verify(&verifier, at: 0, of: T.self)

[hassila]

@mustiikhalil the flatbuffer message will be appropriately aligned internally (and the message is valid), but when reading multiple messages streaming from e.g. a network buffer the starting position for a given message is not necessarily aligned appropriately when they are sequentially laid out in memory - using the loadUnaligned function as suggested allows for direct access of the memory as a flatbuffers message without requiring making a copy (which is one of the main features of flatbuffers!) and is a re ent addition to Swift (5.7 iirc). The same may be an issue if mmaping a file containing flatbuffers messages sequentially laid out. Using load unaligned allows these use cases to work as intended too….

[mr-swifter]

Yes, exactly, thanks @hassila !

I think the test I've added visualise the issue:

func testUnalignedRead() {
...
             let fbb = createMonster(withPrefix: false)
...
             // Unaligned read
             let testUnaligned: () -> Bool = {
                 var bytes: [UInt8] = [0x00]
                 bytes.append(contentsOf: fbb.sizedByteArray)
                 return bytes.withUnsafeMutableBytes { ptr in
                     guard var baseAddress = ptr.baseAddress else {
                         XCTFail("Base pointer is not defined")
                         return false
                     }
                     baseAddress = baseAddress.advanced(by: 1)
                     let unlignedPtr = UnsafeMutableRawPointer(baseAddress)
                     var monster = Monster.getRootAsMonster(bb:
                         ByteBuffer(assumingMemoryBound: unlignedPtr, capacity: ptr.count - 1))

                     self.readFlatbufferMonster(monster: &monster)
                     return true
                 }
             }
...

Here I emulate this, by creating buffer, move it by one byte (like I got from network buffer with such offset) and try to access this correct flatbuffer, but get precondition on first load of UInt32, since starting point is not aligned to uint32 memory layout.

[mustiikhalil]

Makes sense!
I'll review the PR, but I don't think we will need it in the builder, if that makes sense. Since we always check for alignments when we are building the buffer.

[dbaileychess]

FlatBuffers internally aligns the data, and assumes the buffer itself is placed on an aligned boundary. So depending on how the network buffer lays out stuff, it may start on an unaligned boundary. However, that's outside the control of flatbuffers itself, so I would prefer the solution to be outside flatbuffers as well.

I don't know swift, but specifying the buffer from memory should occur outside of flatbuffers and fed into the getRootAs<type>() method. So do any sort of loadUnAlign call there? Correct me if I am wrong on how the swift API works.

[hassila]

Basically swift support accessing the underlying data unaligned (see the very minimal patch using the new swift 5.7 feature for this https://github.com/google/flatbuffers/pull/7641/files) - when using this the swift implementation also handles when the buffer is placed on an unaligned boundary - this is really desirable for any high performance streaming/serialized data as requiring to place the buffer on an aligned boundary forces a copy in such cases, which is unfortunate given flatbuffers fundamental support for zero-copy. Supporting this is basically free and older swift versions would work just as today (i.e. don’t support unaligned access).

[mr-swifter]

I've removed loadUnaligned from builder.

[mzaks]

Strictly speaking [loadUnaligned](https://github.com/apple/swift-evolution/blob/main/proposals/0349-unaligned-loads-and-stores.md) is slower then load as stated in the linked document:

We believe that Swift's goals align well with having the more performant function (aligned load) be the default one.

FlatBuffers first principal is being fast. Bit-packing is not part of FlatBuffers goals. So when multiple buffers are concatenated in one stream, or file, they should be padded. This is IMHO what a high performance application should do if it chose to use FlatBuffers in the first place. Switching to loadUnaligned in func read<T>(def: T.Type, position: Int) -> T will tax every user of FlatBuffers for no benefit.