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

Idea: root versioning with var-uints #7550

Closed
CasperN opened this issue Sep 26, 2022 · 4 comments
Closed

Idea: root versioning with var-uints #7550

CasperN opened this issue Sep 26, 2022 · 4 comments
Labels

Comments

@CasperN
Copy link
Collaborator

CasperN commented Sep 26, 2022

Users have to know what's at the root of their flatbuffer. Sometimes there's a size prefix and I've suggested the root can be a 64bit vector in #7537 (comment). There's also variability in the presence of a file prefix. I think it'd be problematic if the number of "root types" proliferated and users had to keep track of that, so I propose the following scheme so buffers can know about their own roots.

(This supposes we accept my suggestion in #7537 about having a 64-bit-vector-root type)

The first bytes are a variable length unsigned integer. We'll call it v.

  • v & 6 will tell us the size prefix, (this comes after the varuint and may be unaligned)
    • v & 6 == 0 means there is no size prefix
    • v & 6 == 2 means there is a 4 byte size prefix
    • v & 6 == 4 means there's an 8 byte size prefix
    • v & 6 == 6 is interpreted the same as v & 6 == 4
  • v & 8 == 1 if there is a 4 byte file identifier after the root offset
  • v & 16 == 1 means the root offset is to a 64-bit vector and not the normal offset to the root table

This is probably overkill and we'd have to be careful about whether we promise forwards/backwards compatibility and what unknown versions mean, but I'd like to hear what people think.

@dbaileychess
Copy link
Collaborator

Could we give thought on a having it implemented as a normal flatbuffer table instead?

For Example:

table metatable {
   len:int64;
   file_identifier:string;
   root_offset:int64;
   etc...
}

That way we can default them in code to sensible defaults, but allow users to customize them?

The pros are:

  • We can use existing code for accessing these fields
  • Allows future expansion and forwards/backwards compatibility

The cons are:

  • Bloated size, since we now need at least SOffset32 + 2 uint16_t more space for this metatable
  • Not sure how we can implement this seamlessly to work with older data.

@CasperN
Copy link
Collaborator Author

CasperN commented Oct 5, 2022

We'd probably want lazy or shallow verification if we recommend that, since this metadata is most useful when you're not sure you actually have a flatbuffer table. Also, yeah there's nothing that can be done about forwards/backwards compatibility, I think we're cornered on that one

@github-actions
Copy link

github-actions bot commented Apr 5, 2023

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

@github-actions github-actions bot added the stale label Apr 5, 2023
@github-actions
Copy link

This issue was automatically closed due to no activity for 6 months plus the 14 day notice period.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2023
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

2 participants