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

flatbuffers #1063

Closed
wants to merge 1 commit into from
Closed

flatbuffers #1063

wants to merge 1 commit into from

Conversation

jmillan
Copy link
Member

@jmillan jmillan commented Apr 17, 2023

WIP to remove JSON in favour of flatbuffers for as the worker communication format.

  • JSON parsing is CPU intensive.
  • JSON is not type safe.
  • flatbuffers does not parse the buffer. Reading the message takes 0ms.
  • flatbuffers is type safe since code (C++, Typescript, Rust, etc) is autogenerated out of the schema definitions.

Extras that come along with this changes:

  • There is now a single Channel for worker communication.
    • Previously there were 2 (Channel and PayloadChannel)
  • Each message sent Node->Worker requires a single write() call to Channel.
    • Previously 2 calls per message were needed for Channel and 4 for PayloadChannel.
  • Each message send Worker->Node previously executed two extra calls to memcpy that are now executed now.

Feature request for making optional field getters return undefined instead of null if the value is not set.

  • Planus not supporting same table name in different namespaces.
  • flatbuffers (TS) should throw TypeError instead of Error when there is a missing required field. PR

@jmillan jmillan mentioned this pull request Apr 17, 2023
6 tasks
@ibc
Copy link
Member

ibc commented Apr 17, 2023

So is this PR mergeable in v3 without conflicts?

@nazar-pc
Copy link
Collaborator

Technically, yes

@jmillan jmillan closed this Apr 17, 2023
@jmillan jmillan deleted the flatbuffers-new branch April 17, 2023 10:30
@jmillan
Copy link
Member Author

jmillan commented Apr 17, 2023

This branch only contains FBS related changes and those that where only present in flatbuffers branch. So it's definitely more clean than the previous one.

@jmillan
Copy link
Member Author

jmillan commented Apr 17, 2023

Renamed flatbuffers-new to flatbuffers. Final draft PR here #1064

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

Successfully merging this pull request may close these issues.

3 participants