Skip to content

Conversation

ludfjig
Copy link
Contributor

@ludfjig ludfjig commented Aug 20, 2025

We are currently zeroing out some buffers when we do not need to. By removing the extra clear, performance should increase:

Copy-pasted comment from code here

In the happy path we do not need to clear io-buffers from the host because:

  • the serialized guest function call is be zeroed out by the guest during deserialization, see call to try_pop_shared_input_data_into::<FunctionCall>()
  • the serialized guest function result is zeroed out by us (the host) during deserialization, see get_guest_function_call_result
  • any serialized host function call are zeroed out by us (the host) during deserialization, see get_host_function_call
  • any serialized host function result is zeroed out by the guest during deserialization, see get_host_return_value

In addition,this PR fixes an issue when host functions panicked, the associated mutexes were poisoned which prevented making further host calls to the same function. EDIT: this PR does not fix that

Closes #715

@ludfjig ludfjig added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Aug 20, 2025
@ludfjig ludfjig marked this pull request as draft August 20, 2025 22:39
@ludfjig ludfjig force-pushed the no_io_buffer_clear branch 4 times, most recently from 58c76e0 to 48ca86a Compare August 21, 2025 22:34
@ludfjig ludfjig requested a review from Copilot August 21, 2025 22:42
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes performance by only clearing I/O buffers after unsuccessful guest calls, rather than clearing them after every call. Additionally, it fixes issues with poisoned mutexes when host functions panic and adds required system calls to the seccomp allowlist for the Rust panic handler.

Key changes:

  • Conditional buffer clearing to improve performance in the happy path
  • Enhanced panic handling for host functions with proper error propagation
  • Addition of syscalls needed by Rust's panic handler to the seccomp allowlist

Reviewed Changes

Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/hyperlight_host/src/sandbox/initialized_multi_use.rs Added conditional buffer clearing logic and comprehensive test for host function panic scenarios
src/hyperlight_host/src/sandbox/host_funcs.rs Enhanced panic handling to extract and propagate panic messages as proper errors
src/hyperlight_host/src/func/host_functions.rs Fixed poisoned mutex handling to allow recovery from panicked host function calls
src/hyperlight_host/src/error.rs Added new HostFunctionPanic error variant for better error reporting
src/hyperlight_host/src/seccomp/guest.rs Added syscalls required by Rust's panic handler to the allowlist
src/tests/rust_guests/simpleguest/src/main.rs Added test guest function to trigger host function panics for testing

@ludfjig ludfjig force-pushed the no_io_buffer_clear branch from 48ca86a to ed51c21 Compare August 21, 2025 22:44
@ludfjig ludfjig marked this pull request as ready for review August 21, 2025 22:45
danbugs
danbugs previously approved these changes Aug 21, 2025
Copy link
Contributor

@danbugs danbugs left a comment

Choose a reason for hiding this comment

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

LGTM

@ludfjig ludfjig force-pushed the no_io_buffer_clear branch 7 times, most recently from ef9bfd4 to 57f1716 Compare August 28, 2025 22:23
syntactically
syntactically previously approved these changes Sep 1, 2025
Signed-off-by: Ludvig Liljenberg <[email protected]>

Undo stuff that breaks unwinding

Signed-off-by: Ludvig Liljenberg <[email protected]>
Signed-off-by: Ludvig Liljenberg <[email protected]>
@ludfjig ludfjig merged commit 49e248a into hyperlight-dev:main Sep 8, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Call clear_io_buffers only on error conditions

4 participants