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

Remove unnecessary protobuf deserialization/serializing in data node #7681

Closed
pscott31 opened this issue Feb 23, 2023 · 0 comments · Fixed by #7717
Closed

Remove unnecessary protobuf deserialization/serializing in data node #7681

pscott31 opened this issue Feb 23, 2023 · 0 comments · Fixed by #7717
Assignees
Labels
feature new feature to be developed

Comments

@pscott31
Copy link
Contributor

Currently when we get event messages from core we

  • Deserialize them as they come in
  • Serialiaze them again and write them to an event buffer file
  • Read from that file and deserialize them again

It's surprisingly expensive decoding/encoding protobuf messages (e.g. we spend 1/2 of a CPU doing this when the network gets going).

So lets not do this. At the very least

  • Skip the initial Deserialize/Serialize step and just write the bytes we get from the stream to the file

Optional extension project - see how much faffing it would be to only write to the file once an in-memory buffer is full, as-is it's wasted I/O and probably adds a bit of latency to the pipeline [though @ettec would like to keep the option to write to a file for debugging purposes].

Extra extension project - might be some gains to be had by batching writes to the file buffer

Don't spend much time on the extensions though, we haven't noticed the current system actually causing any grief; just feels inefficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new feature to be developed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants