patina_boot: Benchmark execute end to end - #201
Conversation
…ches Benchmark connect_all against synthetic handle topologies of 10/100/1000 handles (stable topology; measured work scales with handle count per convergence pass) and expand_device_path against 4/64/256 filesystem volumes where only the last handle's HardDrive node matches the partial path (worst-case scan). Both reuse the leaked-mock pattern established by bds_phase_composite so per-iteration memory stays flat.
…tables execute() takes the concrete StandardBootServices/StandardRuntimeServices wrappers, so it cannot be driven by the trait mocks. Add benches/support/ fake_tables.rs, which populates full efi::BootServices and efi::RuntimeServices tables with correctly typed stubs: slots the execute flow exercises carry canned behavior (no handles, no variables, no loadable images) and the rest log and return UNSUPPORTED. The execute_e2e bench runs the complete flow per iteration (connect, phase signals, console and boot-option discovery, boot attempts) and asserts the exhausted-error terminal state. Dispatch is the patina MockDxeDispatch. The table factory is a prototype of what a StandardBootServices test factory in patina itself could provide.
There was a problem hiding this comment.
Pull request overview
Adds host-side Criterion benchmarks for patina_boot, including a new end-to-end benchmark intended to exercise SimpleBootManager::execute() without requiring real firmware by providing fake UEFI Boot/Runtime service tables.
Changes:
- Introduces fake
r_efi::efi::BootServices/RuntimeServicestables with stubbed function pointers for host benchmarking. - Extends the
orchestratorbench suite with additional topology benchmarks and a new end-to-endexecute_e2ebenchmark. - Adds a terminal-state assertion in the e2e bench to avoid silently benchmarking an unexpected success path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| uefi/crates/patina_boot/benches/support/fake_tables.rs | Adds fake UEFI Boot/Runtime services tables for running orchestrator logic on host. |
| uefi/crates/patina_boot/benches/orchestrator.rs | Adds new benchmarks (connect_all / expand_device_path topology) and an end-to-end execute_e2e benchmark using the fake tables. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Return a valid empty handle buffer so execute_e2e measures the connect and dispatch loop. Keep benchmark setup outside the timed path and require the expected NotFound terminal state. Assisted-by: GitHub Copilot:gpt-5.6-sol Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53809ea8-9748-401b-a132-83652fd5c7a8
Assisted-by: GitHub Copilot:gpt-5.6-sol Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53809ea8-9748-401b-a132-83652fd5c7a8
Describe the standard UEFI service tables as mocks and centralize their API-coupling contract. Document why every typed slot is populated and how r-efi changes fail at compile time. Assisted-by: GitHub Copilot:gpt-5.6-sol Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 53809ea8-9748-401b-a132-83652fd5c7a8
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (4)
uefi/crates/patina_boot/benches/support/mock_tables.rs:399
bs_create_event_exhas the same issue asbs_create_event: it writes through_a5without checking for null, but the comment implies the write is null-checked. Adding a null check prevents UB if a caller passes a null out-pointer.
// Hand back a dangling non-null event token.
// SAFETY: out-pointers come from the wrapped table caller per the UEFI
// contract; writes are null-checked or spec-required single writes.
unsafe {
*_a5 = 0x1000 as efi::Event;
uefi/crates/patina_boot/benches/orchestrator.rs:245
- The doc comment still references
support/fake_tables.rs, but the benchmark now usessupport/mock_tables.rs(via#[path = "support/mock_tables.rs"]). Updating the reference avoids confusion when someone tries to find the table implementation.
/// True end-to-end bench of `BootOrchestrator::execute()` against fake
/// firmware tables: `StandardBootServices`/`StandardRuntimeServices` wrap
/// stub `efi::BootServices`/`efi::RuntimeServices` tables (see
/// `support/fake_tables.rs`), and dispatch is a no-op mock. The canned
/// behavior presents no handles, no variables, and no loadable images, so
uefi/crates/patina_boot/benches/support/mock_tables.rs:97
bs_create_eventunconditionally writes through_a4but the safety comment claims the write is null-checked. If a caller ever passes a null out-pointer, this is immediate UB. Either add a null check and returnINVALID_PARAMETER, or adjust the safety comment to match reality.
This issue also appears on line 395 of the same file.
// Hand back a dangling non-null event token.
// SAFETY: out-pointers come from the wrapped table caller per the UEFI
// contract; writes are null-checked or spec-required single writes.
unsafe {
*_a4 = 0x1000 as efi::Event;
uefi/crates/patina_boot/benches/support/mock_tables.rs:385
bs_copy_memandbs_set_memare currently no-ops. If any benchmark path (or future extension) relies on these boot services, the behavior will silently diverge from UEFI semantics and can produce misleading benchmark behavior. These are easy to implement correctly withcopy_nonoverlapping/write_bytes.
unsafe extern "efiapi" fn bs_copy_mem(_a0: *mut core::ffi::c_void, _a1: *mut core::ffi::c_void, _a2: usize) {
// no-op
}
unsafe extern "efiapi" fn bs_set_mem(_a0: *mut core::ffi::c_void, _a1: usize, _a2: u8) {
// no-op
}
Design status
Draft; do not merge as the final upstream design. OpenDevicePartnership/patina#1743 removes
StandardBootServicesfrom the Patina component model and introduces granular, mockable UEFI service traits. This PR is retained as a validated v22 benchmark spike. #202 tracks migratingpatina_bootto the new service model and replacing the full boot-service table with generated service mocks.Runtime-variable services are not part of Patina #1743 yet, so the migration must keep that boundary explicit rather than assuming all standard-service access has been replaced.
Summary
SimpleBootManager::execute()pathEvidence
cargo bench --bench orchestrator --no-runsucceeds with the crate's pinned nightly toolchainMock table maintenance
SimpleBootManager::execute()accepts concrete standard-service wrappers, so the mock factory must populate every typed UEFI table slot. Only exercised slots implement behavior; all others returnUNSUPPORTED. The table is centralized in one module, andr-efitable-shape changes fail compilation at that boundary.