Skip to content

Optimize serialization of function calls #444

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ludfjig
Copy link
Contributor

@ludfjig ludfjig commented Apr 29, 2025

  • Removes a vec clone by avoiding calling to_vec on the serialized flatbuffer slice. A downside of this is that we expose an implementation detail ("FlatBufferBuilder"), but I think this is acceptable for FunctionCall, and not visible to users anyway.
  • Removes an unnecessary validation of an encoded flatbuffer slice, by making write_guest_function_call take in a FunctionCall rather than a raw slice.
  • Removes unused function validate_host_function_call_buffer

guest_call_* benchmarks shows rougly -6% execution speedup on my local machine, but will depend heavily on the types of function calls and parameters used

@ludfjig ludfjig force-pushed the avoid_vec_allocation branch from 8226520 to df05f27 Compare April 29, 2025 18:46
@ludfjig ludfjig added kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. area/performance Addresses performance labels Apr 29, 2025
marosset
marosset previously approved these changes Apr 30, 2025
@@ -288,9 +223,40 @@ impl TryFrom<FunctionCall> for Vec<u8> {
},
);
builder.finish_size_prefixed(function_call, None);
let res = builder.finished_data().to_vec();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This to_vec is the clone I was able to avoid, which is the most important part of this PR

simongdavies
simongdavies previously approved these changes Apr 30, 2025
@ludfjig ludfjig dismissed stale reviews from simongdavies and marosset via 9d05679 April 30, 2025 21:25
@ludfjig ludfjig force-pushed the avoid_vec_allocation branch from df05f27 to 9d05679 Compare April 30, 2025 21:25
@ludfjig ludfjig marked this pull request as draft April 30, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/performance Addresses performance kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants