Repository navigation
perf(sdk): stop repeating bundle hashing and teardown state reads - #12974
Merged
Merged
Conversation
Every CLI process verifies the bundle before its first operation. The linux_arm64 bundle lists 365 MB in 12 files, and hashing them one at a time took 190-360 ms per process. Hash them on several threads instead; the first failure in manifest order still decides the error.
Destroy and plan-destroy read every stage's bindings with tofu init and show, then read them again before planning the stage: two more OpenTofu processes per stage, about 150 ms each. Nothing changes that state in between, so plan each stage from the bindings already read.
Contributor
|
v1 documentation preview: https://nvidia-preview-nemoclaw-v1-pr-12974.docs.buildwithfern.com/nemoclaw |
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.
Part of #12882.
Failure
Nearly all lifecycle test time is spent inside SDK and CLI operations. I measured representative lifecycle tests against a locally built
linux_arm64bundle by logging every OpenTofu subprocess and everyProgress::Completedstep. Four tests (fabric_deployment::harness_pi,remote_service::managed_pi_applies_without_generation_and_refused_sandbox_changes_keep_intent,deployment::web_search_cli_export_reapply_and_destroy,deployment::rejected_policy_fails_promptly_with_context_and_allows_recovery_or_destroy) ran 30 SDK or CLI processes and 102.4 s of OpenTofu and bundle work:tofu plantofu plantofu applytofu plan -refresh-onlybundle.verifytofu show -json(bindings, saved plans, readiness)tofu initTwo items in that profile are repeated work:
tofu initandtofu show, then read them again before planning that stage.Decision
Both changes leave results and errors the same.
The largest remaining fixed cost is outside this change. The pinned
kreuzwerker/docker4.6.0 provider waits a fixed 2 s on everydocker_networkread and removal (networkReadRefreshDelayandnetworkRemoveRefreshDelay; upstreammasterstill has the same constants). Runtime plans, refreshes, creating applies, and destroys of a Docker-managed service each pay it. Inremote_service::remote_model_reapply_and_drift_keep_bindings_and_stop_on_observation_failure, 11 waits account for about 22 s of 62 s. Removing it needs a decision on how the network is managed:docker_networkstate to it without recreating the network.Validation
New
bundle::tests::every_listed_file_is_verified_and_the_first_listed_failure_is_reportedpins verification of every listed file and the error order. It passed before and after the change.deployment::destroy_does_not_require_the_inference_credential_or_rewrite_its_referencenow also asserts that plan-destroy and destroy each initialize the stage twice: once to read bindings and once for the teardown graph. It failed before the change (3 initializations) and passes after.Locally: the SDK
bundle::anddeployment::unit tests; lifecycle tests for teardown, Helm recovery, and theremote_service::managed_bearer_*scenarios;cargo fmt --all --check; andcargo clippy --workspace --all-targets -- -D warnings.The same four tests after the change: 96.3 s of OpenTofu and bundle work (was 102.4 s);
bundle.verify1.8 s (was 6.8 s); 49.4 s wall (was 55.7 s).CI lifecycle JUnit reports, run 38086127712, compared with the median of the 9 preceding successful
Rust desired-stateruns (wall seconds / summed test seconds):The linux_arm64 partitions varied by at most 3.7 s across those 9 runs, so their 8 s and 5 s drops are outside the noise. The linux_amd64 partitions varied by up to 31 s, so one run there does not show a change.