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

Zero-Copy Construction into Existing Buffer #6188

Closed
victorstewart opened this issue Oct 18, 2020 · 12 comments
Closed

Zero-Copy Construction into Existing Buffer #6188

victorstewart opened this issue Oct 18, 2020 · 12 comments
Labels

Comments

@victorstewart
Copy link

victorstewart commented Oct 18, 2020

I have a network server with long lived buffers per "logical socket" and want to construct my flatbuffers directly into this memory.

Given that my buffers are "functionally infinite" compared to the size of flatbuffers I am constructing, am I correct that if I subclass Allocator, implement uint8_t *allocate(size_t size) to simply return the tail of my buffer, and make void deallocate(uint8_t *p, size_t) a no-op, and set initial_size to the giant amount of free space in the buffer that it will "just work"?

I know this is a bit of a hack lol.

@aardappel
Copy link
Collaborator

Yup, that should work, and that kind of use case is intended.

@victorstewart
Copy link
Author

victorstewart commented Oct 19, 2020

would you be open to a PR to make this behavior explicitly/simply usable via the library for all?

like write a new Builder subclass that takes a buffer pointer and fixed capacity in its constructor (and/or via a function so you can make the builder static and rebase it on every usage).

@victorstewart
Copy link
Author

also once you Finish() the builder does the buffer now hold only the constructed flatbuffer and no more scratch / wasted head bytes right?

i haven't fully unknotted the code yet lol.

because I want to directly push the buffer written into out on the wire.

@aardappel
Copy link
Collaborator

Rather than a FlatBufferBuilder subclass we could supply a predefined FixedAllocator or something.. thing is, the question is what kind of error handing should happen if the fixed buffer runs out.. so maybe easiest to keep it as-is.

When you Finish that extra data is now not needed anymore, but of course they may still be in the buffer. The buffer needs to be big enough for the FlatBuffer data and the scratch space usage to work.

@victorstewart
Copy link
Author

victorstewart commented Oct 19, 2020

we could just return an allocation failure if the buffer capacity is exhausted. or... it could silently fail and make note of it in a comment... "user beware".

i see. so at the moment, you'd still have to do a memmove to eliminate the scratch. (that's what i thought from reading the code but just wanted to confirm).

so maybe I could change it so the scratch is stored in a separate internal buffer, so that once you Finish a buffer could truly be pushed out the wire zero-copy (or move).

what do you think?

(i'm using registered buffers with io_uring so it truly is a zero copy paradigm).

@aardappel
Copy link
Collaborator

Even if you didn't have the scratch, the buffer is being built back to front, so you typically always have allocator slack at the front. In fact, the whole point of the putting the scratch buffers there is that typically that space is there anyway.

Why do you need a memmove? Can you not pass an arbitrary part of that buffer to whoever needs it? If it's because there's a header before the buffer, can you not move that header instead (assuming its smaller) ?

@victorstewart
Copy link
Author

victorstewart commented Oct 22, 2020

well the reader expects the next message to be at the next 4 byte aligned address (of course that could change). but if the flatbuffer were built directly into the registered buffer, you'd have to memmove the buffer down... to properly align it.

and if the scratch were there you'd have to memset that head space to 0.

essentially my aim here was to ascertain if there's anyway to construct the flatbuffer in place with 0 moves 0 copies 0 memsets post construction.

but i guess the fact that the buffer is built backwards, thus always lingering head space, unless i change the alignment semantics, the minimum is a memmove. (thus no point interfering with the existing scratch space mechanics).

given that, are you open to adding a FixedAllocator to the library? Assuming we agree on the aforementioned problem points.

@AachoLoya
Copy link

@victorstewart Did you figure out a solution? Im in the same situation as you.

@AachoLoya
Copy link

Even if you didn't have the scratch, the buffer is being built back to front, so you typically always have allocator slack at the front. In fact, the whole point of the putting the scratch buffers there is that typically that space is there anyway.

Is it possible to reverse this behavior, so that the buffer is built front-to-back? so that the buffer starts from index zero. That will make it easier to just provide a custom allocator that is based on an existing memory location.

@aardappel
Copy link
Collaborator

Is it possible to reverse this behavior, so that the buffer is built front-to-back?

In theory yes, in practice no, since that would be incompatible with existing buffers (if we want to keep offsets unsigned for cycle protection), and it would be an enormous amount of work getting all code to support it.

@AachoLoya
Copy link

In theory yes, in practice no, since that would be incompatible with existing buffers (if we want to keep offsets unsigned for cycle protection), and it would be an enormous amount of work getting all code to support it.

Why not make it a optional flag when using schema compiler? That won't break existing applications but new applications can take advantage of low-to-high memory structure and custom allocator for improved performance.

@github-actions
Copy link

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

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

No branches or pull requests

3 participants