Repository navigation
nvx-rs: delete the exec engine and its guest binding (SL6.11a) - #1506
Draft
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Draft
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Pedro Henrique Penna (ppenna) wants to merge 1 commit into
Conversation
No NVX binary runs an execution through the exec engine any more: the runtime adapter, whose host side was the engine's Peer and whose guest worker bound its Runner to the guest's executions, left NVX in SL5.2, and nvx-init and the managed agent drive their executions themselves. So nvx::exec, the credit and stdio engine of an execution, leaves, with Runner, Peer, their messages and state machines, the credit pools, Key, Limits, Timers, Window, Outcome, LaunchFailure and the rest of its public API, and so does its guest binding, guest::exec::binding, with Binding, its Event, and the signal path through which Binding::signal passed a caller's signal on to its execution. Their tests go with them: the 56 of src/exec/tests/, the 27 of guest/exec/binding/tests.rs, and the doc test of the engine's module. The binding was the only caller of Execution::output_hung_up and Execution::release_cgroup, and its tests the only users of the test helpers Command::spawn_holding_report and PreparedChild::report_writer_copy, so they go too, as the compiler's dead-code check requires. Execution::signal stays with guest::exec, which SL6.11b deletes with the other process modules. The rustdoc example of LaunchStage::from_code read a launch failure from the engine's Outcome, so it goes too. The unit test launch_stages_keep_their_numbers still checks that from_code maps every stage's number back to the stage, and 0 to none. The crate's rustdoc, that of guest, guest::exec and guest::spawn, the nvx README's layout, module table, sections and testing notes, the guest-agent guidelines, a comment of check-nvx-rs and both public-API snapshots drop the engine and the binding. The module diagrams' page drops the engine's section and diagram, whose link to the deleted module would break; CI draws the other diagrams again. The case matrix retires the engine's doc test, gives from_code's its coverage, and names the doc tests of the crate, guest and guest::exec by the lines to which they move. Part of #1123 (SL6.11a). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
This pull request changes existing testsIt modifies or removes 207 existing test asset(s), listed below. Review each with care: a change to a test can hide a regression that the test caught. See doc/contribute.md#changes-to-existing-tests. Test code
... and 157 more, which the job's summary lists. Relocated unchanged: 0; added: 0. Compared the merge base |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the removal of the obsolete exec engine and guest binding.
No open issues 🎉
What changed in this PR
Removes the obsolete exec engine and its guest binding for SL6.11a of #1123, following removal of their last consumer in #1499. This advances NVX’s library-slimming work while leaving the guest process modules for SL6.11b.
Changes:
- Deletes the engine, guest binding, associated tests, and helpers used only by the binding.
- Removes exports and public-API snapshot entries for the deleted modules.
- Updates documentation, the test-fate matrix, and a CI comment without changing CI commands.
| File | Description |
|---|---|
nvx-rs/src/lib.rs |
Removes the engine export and documentation references. |
nvx-rs/src/guest/sys/spawn/tests.rs |
Removes the binding-only report-writer helper. |
nvx-rs/src/guest/spawn/tests.rs |
Removes the binding-only delayed-report spawn helper. |
nvx-rs/src/guest/spawn.rs |
Removes binding references and the engine-dependent doctest. |
nvx-rs/src/guest/mod.rs |
Removes engine and binding documentation references. |
nvx-rs/src/guest/exec/binding.rs |
Deletes the guest binding and its signal forwarding. |
nvx-rs/src/guest/exec.rs |
Removes the binding declaration and its private helpers. |
nvx-rs/src/exec/tests/truncation.rs |
Retires truncation and settlement tests with the engine. |
nvx-rs/src/exec/tests/streams.rs |
Retires engine stream-ordering tests. |
nvx-rs/src/exec/tests/races.rs |
Retires engine terminal-race tests. |
nvx-rs/src/exec/tests/mod.rs |
Deletes the simulated engine test harness. |
nvx-rs/src/exec/tests/memory.rs |
Retires engine bounded-memory tests. |
nvx-rs/src/exec/tests/liveness.rs |
Retires engine progress and fairness tests. |
nvx-rs/src/exec/tests/keys.rs |
Retires execution-key and late-message tests. |
nvx-rs/src/exec/tests/credit.rs |
Retires engine credit-accounting tests. |
nvx-rs/src/exec/peer.rs |
Deletes the requesting-side engine state machine. |
nvx-rs/src/exec/credit/sender.rs |
Deletes sender-side credit accounting. |
nvx-rs/src/exec/credit/receiver.rs |
Deletes receiver-side credit accounting. |
nvx-rs/src/exec/credit/inbox.rs |
Deletes the engine’s bounded input queue. |
nvx-rs/src/exec/credit.rs |
Deletes shared credit and queue helpers. |
nvx-rs/README.md |
Removes engine and binding layout, API, and test descriptions. |
nvx-rs/public-api/windows.txt |
Removes the engine’s public-API entries. |
nvx-rs/public-api/linux.txt |
Removes engine and binding public-API entries. |
doc/modules/README.md |
Removes the engine diagram section and source link. |
doc/modules/nvx-exec.svg |
Deletes the obsolete engine diagram. |
doc/guest-agent-guidelines.md |
Stops describing the engine as retained code. |
doc/acceptance-case-matrix.md |
Records doctest retirement, replacement coverage, and moved lines. |
.github/actions/check-nvx-rs/action.yml |
Updates the no-features comment; commands remain unchanged. |
Review effort: Balanced
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Leaving code only. This PR changes only code that this effort removes from NVX: the exec engine (
nvx::exec) and its guest binding (guest::exec::binding), with the edits that remove references to them. Nothing that stays changes, and no later PR uses what it changes, so the size limits for kept code don't apply.Part of #1123 (SL6.11a, "The exec engine is gone"). Parent #1087, umbrella #1076.
Stacked on #1499 (SL5.2), which deletes the runtime adapter, the engine's last user: this PR's base is SL5.2's branch,
agents/slim-sl5-2-remove-adapter, until #1499 merges.What and why
No NVX binary runs an execution through the exec engine any more. Its last user was the runtime adapter, whose host side was the engine's
Peerand whose guest worker bound itsRunnerto the guest's executions throughguest::exec::binding; #1499 (SL5.2) deletes it. nvx-init runs its executions with its own plain-process core, and the managed agent drivesguest::exec::Executiondirectly. So, as the Scope of SL6.11a says, the engine and its binding leave as complete units:nvx::execgoes:nvx-rs/src/exec/(mod.rs,runner.rs,peer.rs,credit.rsandcredit/{inbox,receiver,sender}.rs), withRunner,Peer, their messages and state machines, the credit pools,Key,Limits,Timers,Window,Outcome,LaunchFailureand the rest of its public API, andsrc/exec/tests/.guest::exec::bindinggoes:nvx-rs/src/guest/exec/binding.rs, withBinding, itsEvent, and the signal path attached to the engine,Binding::signal, which passed a caller's signal on to its execution; andguest/exec/binding/tests.rs.nvxa library of mechanisms and value types #1076's rules and the compiler's dead-code check require (clippy-D warningsfails on them otherwise):Execution::output_hung_upandExecution::release_cgroup,pub(crate)methods that 3a6c959 added toguest::execfor the binding, which was their only caller; and the test helpersCommand::spawn_holding_report(guest/spawn/tests.rs) andPreparedChild::report_writer_copy(guest/sys/spawn/tests.rs), which 4178009 added for the binding's tests, their only users.guest::{exec, spawn, process, cgroup, entry}, includingExecution::signaland the signal helpers ofguest::systhat it uses, andLaunchStage::codeandfrom_code, whichguest::spawnitself still uses.guest,guest::execandguest::spawnno longer mention the engine or the binding. The rustdoc example ofLaunchStage::from_coderead a launch failure from the engine'sOutcome, so it goes;guest/spawn/tests.rs::launch_stages_keep_their_numbersstill checks thatfrom_codemaps every stage's number back to the stage, and 0 to none. The nvx README drops the engine's and the binding's rows of the module table, their sections, theexec/entry and the binding's phrase of the layout, and its note on their tests. The guest-agent guidelines no longer say that the crate keeps the engine, andcheck-nvx-rs's comment no longer says that the library without features holds it. The public-API snapshots lose 504 lines: the 237 items ofnvx::execon each platform, and the 30 ofguest::exec::bindingon Linux.doc/modules/README.mdloses the engine's section, andnvx-exec.svggoes; see Decisions.doc/acceptance-case-matrix.mdretires the engine module's doc test, givesLaunchStage::from_code's its coverage (rewritten), and names the doc tests of the crate,guestandguest::execby the lines to which they move; the Docs row of the summary becomes 36: 33 kept, 2 rewritten, 1 retired. No row names a test of the engine or the binding as coverage (git grepof each removed name in the matrix).Decisions
Choices that this PR makes beyond deleting the two modules, and why:
Executionmethods and the two test helpers above. The guest lint (check-guest-binaries, clippy-D warnings) refuses them once the binding goes, and nvx-rs slimming workstream: makenvxa library of mechanisms and value types #1076 deletes what a deletion leaves unused in the same PR.doc/modulesis edited by hand, only as far as the checks need. CI draws the module diagrams, anddoc/ci.mdleavesdoc/modulesto it. But the page linkednvx-rs/src/exec/mod.rs, whichcheck_markdown_links.pyrefuses once the file is gone, andtest_module_diagrams.pyrequires the directory to hold exactly the page and the images that it shows. So this PR removes only the engine's section of the page andnvx-exec.svg, as the generator would. The overview, the dependencies andnvx::gueststill show the engine and the binding:module-diagrams.ymldraws for pull requests intodevand for runs ofdev, and publishes ondevalone, so they stay stale onfeature/nvx-refactoruntil it merges intodevand the next nightly run redraws them. That is tracked outside this PR.LaunchStage::from_code's doc example retires as rewritten. It read a launch failure from the engine'sOutcome(use nvx::exec::{LaunchFailure, Outcome}), so it could not stay, and a new example for a module that SL6.11b deletes would add code that leaves. The case matrix marks it rewritten, covered byguest/spawn/tests.rs::launch_stages_keep_their_numbers, which checksfrom_codefor every stage's number and for 0.Execution::signalstays, for SL6.11b. The signal path thatdevadded to the engine and its binding isBinding::signal(d778662), which goes.Execution::signal(9aba834), and the signal helpers ofguest::systhat it uses (32d76f9), belong toguest::exec, one of the process modules that SL6.11b deletes with their remaining callers. No NVX code calls it now, but it is public API of a module that stays here, so the dead-code check doesn't flag it, and its tests inguest/exec/tests.rsstay with it.LaunchFailure's, naming the deletedguest::exec::binding. The engine and its binding leave as one unit, so one commit keeps every commit consistent.Every removed test, and its fate
All retire with the code that they test: what they check leaves NVX.
nvx-rs/src/exec/tests/:credit10,keys4,liveness6,memory3,races10,streams8,truncation15nvx-rs/src/guest/exec/binding/tests.rsa_signal_that_the_program_handles_reaches_the_runner_only_as_the_program_s_endtestednvx-rs/src/exec/mod.rs - exec (line 113)nvx-rs/src/guest/spawn.rs - guest::spawn::LaunchStage::from_code (line 201)launch_stages_keep_their_numberschecksfrom_code; the engine's launch failure and outcome, which the example read, leaveThe 83 removed unit tests, by name
nvx-rs/src/guest/exec/binding/tests.rs(27):output_beyond_its_windows_arrives_whole_and_the_outcome_is_lossless,input_longer_than_its_window_reaches_the_program_intact,a_program_that_closes_its_input_early_stops_the_peer_s_source,an_adopted_program_runs_bound_as_a_spawned_one_does,a_failed_launch_stage_reaches_the_peer_by_its_number,the_input_closes_with_its_last_byte_while_output_waits_to_be_returned,a_write_that_fails_closes_the_program_s_input,descendants_and_pipe_writers_settle_before_reuse_in_a_cgroup_that_the_test_plays,the_settlement_kills_what_the_program_leaves_behind_in_a_cgroup_that_the_test_plays,the_settlement_begins_as_the_program_ends_while_a_descendant_holds_its_output,descendants_setsid_and_pipe_writers_settle_before_reuse_in_a_real_cgroup,a_cleanup_failure_fences_the_execution,an_unrelated_child_s_status_stays_with_its_owner,a_caller_that_reaps_hands_the_program_s_status_over,a_signal_that_the_program_handles_reaches_the_runner_only_as_the_program_s_end,the_program_s_end_reaches_the_runner_before_queued_output_whoever_collects_its_status,the_program_s_end_reaches_the_runner_before_its_timeout_in_a_first_call_after_it,the_program_s_end_reaches_the_runner_before_its_timeout_while_its_launch_report_is_late,the_runner_gives_up_in_time_on_a_launch_report_that_a_process_holds_after_the_program_s_end,a_stalled_sink_stalls_only_its_own_execution_and_endings_still_finish_in_time,a_call_takes_a_bounded_number_of_steps_and_asks_to_be_called_again,a_call_writes_a_bounded_number_of_chunks_of_input_and_asks_to_be_called_again,output_flows_while_the_caller_hands_over_input_before_every_call,a_pipe_writer_that_escapes_the_kill_fences_its_execution_in_time_and_holds_no_other,an_execution_that_does_not_settle_gives_up_in_time_on_the_output_that_a_descendant_holds,an_execution_that_does_not_settle_loses_nothing_of_the_output_that_no_process_holds,a_due_settlement_check_runs_while_output_drains_slowlynvx-rs/src/exec/tests/credit.rs(10):each_receiver_first_grants_its_whole_window_once,a_byte_window_admits_data_up_to_its_edge_and_no_further,data_messages_carry_at_least_a_byte_and_at_most_a_chunk,the_runner_sends_the_most_that_its_credit_a_chunk_and_the_buffer_allow,a_frame_window_charges_one_unit_per_message_whatever_its_length,a_shared_pool_charges_both_streams_and_answers_to_its_own_name,invalid_grants_and_over_credit_are_refused_and_change_nothing,maximal_windows_and_chunks_neither_overflow_nor_admit_more_than_the_window,partial_consumption_returns_only_what_the_sink_took,the_peer_sends_input_within_the_runner_s_creditnvx-rs/src/exec/tests/keys.rs(4):messages_of_an_earlier_generation_are_stale_and_change_nothing,messages_of_another_execution_or_a_later_generation_are_refused,messages_after_the_execution_ended_are_late,a_cancellation_after_a_termination_began_is_latenvx-rs/src/exec/tests/liveness.rs(6):a_stalled_sink_with_an_exhausted_window_stalls_only_its_own_execution,endings_finish_by_their_deadlines_while_a_sink_is_stalled_and_another_execution_streams,a_full_transport_holds_back_only_the_messages_that_wait_for_it,input_credit_takes_turns_with_the_runner_s_other_messages,a_stream_s_end_goes_out_while_the_other_stream_s_output_keeps_flowing,output_credit_takes_turns_with_the_peer_s_input_and_its_other_poolnvx-rs/src/exec/tests/memory.rs(3):a_stream_far_longer_than_its_window_arrives_whole_in_bounded_memory,a_shared_pool_of_frames_carries_both_streams_whole_in_bounded_memory,input_longer_than_its_window_arrives_wholenvx-rs/src/exec/tests/races.rs(10):an_exit_racing_each_ending_gives_one_terminal_outcome,an_exit_that_arrives_once_the_timeout_passed_but_before_a_tick_stops_the_timeout,the_first_cause_wins,the_peer_finishes_once_whether_its_cancellation_or_the_terminal_outcome_comes_first,a_peer_detached_before_the_terminal_outcome_finishes_on_its_own,ended_streams_stay_truncated_after_a_peer_detaches_without_the_terminal_outcome,ended_streams_stay_truncated_after_a_peer_gives_up_without_the_terminal_outcome,a_terminal_outcome_keeps_precise_truncation_even_if_the_peer_then_detaches,the_runner_s_messages_after_the_peer_detached_are_late_while_its_sink_drains,the_runner_s_messages_after_the_peer_gave_up_on_it_are_latenvx-rs/src/exec/tests/streams.rs(8):each_stream_keeps_its_order_and_its_end_follows_its_last_byte,the_input_ends_only_after_every_accepted_byte,a_workload_that_closes_its_input_early_never_leaves_the_source_waiting,input_after_the_workload_s_input_closed_is_dropped_within_the_window,closing_the_input_keeps_the_output_flowing,a_failed_launch_sends_no_start_and_reports_why,a_second_start_end_or_terminal_is_out_of_order,a_start_after_the_terminal_outcome_of_a_failed_launch_is_out_of_ordernvx-rs/src/exec/tests/truncation.rs(15):a_workload_that_ends_on_its_own_loses_no_byte_however_slow_the_sink,a_termination_drains_pending_output_for_its_grace_then_marks_the_loss,a_termination_whose_pending_output_drains_within_the_grace_loses_nothing,a_failed_read_truncates_its_stream_without_a_cause,a_failed_or_late_settlement_fences_the_execution,an_execution_that_does_not_settle_gives_up_on_the_streams_still_open_after_its_end,an_execution_that_does_not_settle_loses_nothing_of_the_streams_that_hung_up,a_settled_execution_waits_for_its_sink_however_long_its_streams_stay_open,the_runner_gives_up_on_a_launch_result_that_a_process_holds_back_after_the_workload_s_end,a_launch_result_after_a_termination_s_give_up_counts_for_nothing,a_launch_result_that_arrives_after_the_workload_s_end_but_in_time_still_counts,a_settlement_that_the_runner_did_not_request_counts_for_nothing,the_runner_gives_up_on_a_workload_that_will_not_end_and_fences_it,a_peer_that_ended_the_execution_gives_up_on_a_silent_runner,the_peer_s_own_timeout_cancels_and_names_itselfSize
Measured against the stack base,
agents/slim-sl5-2-remove-adaptera2fadde7:nvx::exec, 372 of the binding, 10 ofguest/exec.rs(pub mod binding;and the two methods) and 1 oflib.rs(pub mod exec;).nvx-rs/src/lib.rsandnvx-rs/src/guest/exec.rs. Three more change only comments:nvx-rs/src/guest/mod.rs,nvx-rs/src/guest/spawn.rsand.github/actions/check-nvx-rs/action.yml, so 5 under a strict reading. The 17 files that it only deletes don't count, nor do the two test files, the docs, the snapshots and the generated diagrams.Commits
nvx-rs: delete the exec engine and its guest bindingNot in this PR
Deleting the guest process modules,
Execution::signal, the signal helpers ofguest::sys, the spawn child and the per-exec OS code (SL6.11b); re-homing the engine, the binding or the signal path; any replacement execution API; redrawing the module diagrams; and any change to code that stays.Validation
local_gates.py(dev's copy,--filesof this change) selects test-changes, markdown-links, test-layout, the nvx-rs gates and nvx-cli on Windows, and also the public API, macOS and guest-binary gates on Linux; its adapter gates no longer exist on this branch. Each ran from this branch's own action definitions. The Linux checks ran on an earlier push of this branch,9b30acaa, whose tree differs from this head's only in one line of the case matrix.Locally on Windows, Rust 1.95.0,
NVX_BUNDLE=skip:cargo fmt --all --checkcargo clippy --lockedwith--all-targets --all-features,--lib,--lib --no-default-features,--lib --no-default-features --features serde,--lib --bins --features cli,--lib --bins --no-default-features --features testing, each-- -D warningsx86_64-unknown-linux-gnu(--all-targets --all-features,--lib --no-default-features) and forx86_64-unknown-linux-musl --no-default-features --features guest --all-targets,-D warningsoutput_hung_up,release_cgroup,spawn_holding_reportandreport_writer_copyRUSTDOCFLAGS="-D warnings" cargo doc --locked --no-deps --all-features, native and--target x86_64-unknown-linux-gnucargo test --locked --all-features,cargo test --lockedcargo build --locked --lib --no-default-features,cargo test --locked --lib --no-default-featurescargo +1.89 check --locked --all-targets --all-featurescheck-nvx-rs's bundling check: its synthetic release, thencargo test --locked --lib --features bundled artifactsvalidate-nvx's Windows step, less itspip install:python -m compileall -q scripts,python -m unittestof its 22 modules,test_adversarial.py,test_hosts.pyand thenvx.py --helpcheckspython scripts/public_api.py(after--update)python scripts/check_markdown_links.py,python scripts/check_test_layout.pyOn Linux,
nvx-mshv-test(Azure Linux 3, kernel 6.18.34, x86-64 virtual machine), in a temporary worktree of this branch, Rust 1.95.0,NVX_BUNDLE=skip:cargo fmt --all --check, and the sixcargo clippylints ofcheck-nvx-rs,-D warningscargo test --locked --all-featurescargo test --lockedcargo build --locked --lib --no-default-features,cargo test --locked --lib --no-default-featuresRUSTDOCFLAGS="-D warnings" cargo doc --locked --no-deps --all-featurescargo +1.89 check --locked --all-targets --all-featurescargo check --locked --all-targets --all-features --target aarch64-apple-darwinhost/templatetests, which this PR doesn't touchpython3 scripts/public_api.pypython3 scripts/nvx.py check-guest-binaries(musl clippy and tests of theguestfeature)python3 scripts/nvx.py build-guest-binaries,test-guest-binariesNo microVM scenario ran: no binary used the engine or the binding, so no runtime path changes. Left to CI: the Windows and Linux nvx-rs jobs on this head, and the jobs that the change classifier selects.
Changes to existing tests
python3 scripts/check_test_changes.py --base origin/agents/slim-sl5-2-remove-adapter, with this branch's script and withdev's, lists 207 changes to existing tests, none relocated or added (1652 againstfeature/nvx-refactor, with SL5.2's 1445):Command::spawn_holding_reportinnvx-rs/src/guest/spawn/tests.rsandPreparedChild::report_writer_copyinnvx-rs/src/guest/sys/spawn/tests.rs. No test ofguest::spawnorguest::sys::spawnchanges otherwise.