Repository navigation
perf: cut the unit-test step's time on CI runners - #14048
Merged
Merged
Conversation
Projecting a sandbox's Fabric configuration and building authoring capabilities only read the bundled catalog, but each call copied every adapter descriptor and dropped the copy. In unoptimized test builds the copy took most of for_sandbox's time, which onboarding and journey resolution run on every answer. Add FabricCatalog::bundled_ref, which returns the parsed catalog itself, and use it in those two readers. bundled() still returns an owned copy. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
schema_accepts built a validator for every check. Authoring checks the same adapter and SDK field schemas again on every answer, and compiling a schema costs far more than validating a value against it. Keep compiled validators, and schemas that fail to compile, keyed by the schema's JSON text. The cache empties itself at 256 entries, so schemas that images advertise cannot grow it without bound. Results are unchanged: the same text compiles to the same validator. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Each answer resolves the journey before and after the change, and the caller resolves the answered state again for the next question. Every resolution assesses the authored values, and assessing serializes them, parses the result as YAML, and validates the SDK document. In unoptimized test builds that parse took half of each resolution. The assessment depends only on the values, so keep recent assessments keyed by the values' JSON text, the same text the SDK parses. The cache empties itself at 32 entries. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collecting deployment questions checks every authored path against the deployment regions, and formatted two strings per region for each check. Compare the path's remainder after the prefix instead. The same paths match. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Managed gateway, Kubernetes gateway, and Docker storage specs checked their names, owners, and generations by compiling a regular expression for every check. Compiling a pattern costs more than matching it, and compiles and observations check every managed resource; in unoptimized test builds compiling took about a tenth of compiling a deployment. Keep each constant pattern compiled, in the SDK and in nemoclaw-docker's storage checks. The patterns and their results are unchanged. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Answering a deployment field resolved the unchanged journey a second time to check that the question was still active. The answer already holds that resolution: nothing changes the journey before the check. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
release_sized_sboms_are_admissible spent nearly all of its 1.4 s, and up to 4.7 s on four CPUs, serializing its 70 MB comment through unoptimized serde_json. Splice the comment into the fixture document's text instead. The assessment reads the same document, and the test still fails when the file limit is below the comment's size. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
progress_arrives_before_exit_and_cancellation_still_works took 5 s whenever its shell had forked sleep but sleep had not started when the interrupt arrived: sleep kept running, the shell waited for it, and the run waited out its cancellation grace period before killing them. It took 5 s on every run limited to one CPU, and in a full test run limited to four. Have the shell exec sleep, so the interrupt reaches the command it runs. The test still checks that progress arrives before cancellation and that cancelling returns Cancelled. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Answering a question resolves the journey before and after the change, and the frontend resolves the answered journey again for the next question, so each answer resolved the same run two or three times. A resolution depends only on the run and the capabilities. Keep the four latest resolutions on each thread, and reuse one when the run's fields are equal and the capabilities are the same. Capabilities cannot change after construction, so each construction gets an identity that its copies share and that resolutions are keyed by. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
input_schema() generated the SDK input schema on every call, and the SDK's validators, partial document assessment, and authoring's field lookup each kept their own copy, so a process that resolves a journey generated it three times. Unoptimized, generating it took about a third of a short journey test. Add input_schema_ref, which generates the schema once and returns it, and use it in those three places. input_schema() still returns an owned copy. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
schemars 1 schemas already hold a serde_json value, but the SDK serialized each generated schema into a new value. Unoptimized, that copy took two thirds of generating the configuration schema, which every process that parses or validates a document does twice. Take the value with Schema::to_value instead. The values are equal: serde_json keeps object keys sorted here, so the keyword order Schema's serializer applies does not reach the value, and cargo run -p nemoclaw-build -- schema --check finds no drift. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
The runtime, vLLM, and Ollama schemas and the sandbox policy shape serialized each schemars schema into a new serde_json value, though the schema already holds one; take it with Schema::to_value instead. The values are equal, as for the configuration schema. Every sandbox's OpenTofu inputs also derived the policy shape from a freshly generated schema. Derive it once per process. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
Authoring session — opening
|
cv
enabled auto-merge (squash)
November 1, 2026 06:13
Contributor
|
v1 documentation preview: https://nvidia-preview-nemoclaw-v1-pr-14048.docs.buildwithfern.com/nemoclaw |
wscurran
added
area: ci
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.
Summary
Cuts the time of
cargo ci's unit-test step so it stays well under its 45 s budget on thelinux_amd64runner, without raising any budget.Related Issue
The test step failed its budget on several recent PRs (#14037, #14038, #14044) at 45.3 to 49.1 s, then passed on reruns at 33.9 to 44.0 s.
Changes
Twelve commits, each keeping results the same:
Schema::to_value()instead of a serialize round trip.release_sized_sboms_are_admissiblewrites its 70 MB comment directly instead of through unoptimizedserde_json(it still fails when the limit is lowered);progress_arrives_before_exit_and_cancellation_still_worksrunsexec sleepso an interrupt can't leavesleeprunning for the 5 s grace.Failure, Decision and Validation
Failure. CI's
linux_amd64runner has 4 vCPUs running 8 test threads, so the step is CPU-bound, and the suite sat at the 45 s budget: runner variance alone failed PRs.AGENTS.mdsays to make slow tests faster rather than raise budgets.Decision. Remove repeated work that tests exercise heavily, mostly in onboarding and journey resolution, rather than weaken tests. The riskiest change is the resolution cache in
JourneyState::resolve; it relies onCapabilitiesnever changing after construction, which its documentation states. It alone saves about 12% of journey CPU, so it can be dropped if reviewers prefer.Validation. Measured with the
cinextest profile pinned to 4 CPUs (taskset -c 0-3) against a cleanorigin/v1: 32.2 s to 22.2 s wall and 251 to 171 s summed. The onboarding TUI tests went from 57.1 to 17.4 s summed. Unrestricted on 20 cores: 13.6 to 11.4 s wall.cargo cipasses (1805 tests, 162 lifecycle tests); every commit was checked with clippy,cargo fmt --checkand the full suite. Live suites weren't run, since nothing changes images or Kubernetes behavior.Type of Change
Verification
cargo ciAI Disclosure
Signed-off-by: Carlos Villela cvillela@nvidia.com