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

[Swift] Loading unaligned flatbuffers objects #45276

Open
mustiikhalil opened this issue Jan 16, 2025 · 3 comments
Open

[Swift] Loading unaligned flatbuffers objects #45276

mustiikhalil opened this issue Jan 16, 2025 · 3 comments

Comments

@mustiikhalil
Copy link

mustiikhalil commented Jan 16, 2025

Hello,

I'm the maintainer of the flatbuffers swift port, and we have a couple of changes within the pipeline google/flatbuffers#8484 (comment) that might be important for arrow.

TLDR: We refactored the implementation of a bytebuffer to directly read from [Data, UInt8, ContiguousBytes, pointers] aka without copying the data from those objects into our own internal storage. And we want to know if this will break anything, and if we need to return the flag allowReadingUnalignedObjects since in theory the first 4 bytes of the and byte buffer should be a valid root for a Flatbuffer object even if unaligned. One way to say that this byte buffer has a valid flat buffer object is to use the verifier before reading any data

Wasn't sure where to tag this, so sorry in advance

Component(s)

Swift

@kou
Copy link
Member

kou commented Jan 17, 2025

Cc: @abandy

@abandy
Copy link
Contributor

abandy commented Jan 17, 2025

HI @mustiikhalil and @kou,

I hope all is well.

Nice, good to remove unnecessary copying! I will try to pull your changes down and run the tests by end of next week. Even if these changes cause an issue, Swift Arrow should be fine as it is bound to a version of the flatbuffers, so checking in your changes shouldn't break Swift Arrow right away. Thanks for checking!

Thank you

@mustiikhalil
Copy link
Author

mustiikhalil commented Jan 17, 2025

That's perfect!

1- In theory, it should be alright since most of the time when creating a flat buffer object we used to copy the unaligned data into the ByteBuffer. And then read from it, but now we are just reading the unaligned data directly.

2- The only downside to all of this is that now reading would take a bit longer compared to earlier 47 ns, compared to 4 ns but it's safer and it works. If you still want the speed you can always use the copying method since we kept that, or use the assumeMemoryBound api, but then for safety its better that you don't return the bytebuffer, but do all the operations within the clousre

/// This will copy the data and create a byte buffer that will have 7/9 ns of reading
data.withUnsafePointer {
  ByteBuffer(copying...: $0)
}

or

/// This will use the data pointer directly and create a byte buffer that will have 7/9 ns of reading
data.withUnsafePointer {
  // do everything you need with flatbuffers here
  ByteBuffer(assumingMemoryBound: $0)
}

or

/// This will be reduced performance but in the grand scheme of things, when having data as 1gb, its okay to be slower 
ByteBuffer(data: data)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants