Skip to content

feat: bound pending native execution work - #82

Draft
hsliuustc0106 wants to merge 3 commits into
mainfrom
codex-bounded-native-admission
Draft

hsliuustc0106 wants to merge 3 commits into
mainfrom
codex-bounded-native-admission

Conversation

@hsliuustc0106

@hsliuustc0106 hsliuustc0106 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

The shared serial scheduler previously allowed unlimited pending execution units. Bound waiting and dispatched units together (64 per loaded executor by default), configured with OMNI_NATIVE_MAX_PENDING. Full capacity immediately returns a typed overload error without dispatch; both native workers map it to HTTP 503 using their existing detail/error response shape.

Cancelling queued work returns capacity; dispatched work retains both permits and captured model resources until completion. FIFO execution remains serial. Cua-S1 counts individual question forwards, so a later overload fails the whole request without partial answers; Open-Jev counts complete requests. This bounds execution units, not ingress or prepared-token memory.

Update runtime/architecture/model contracts, recipes, frontend documentation and contributor/review skills. Add shared CPU tests and preserved actual-run evidence. Token budgets and batch-capable execution remain later work.

Test Plan

System1-Omni Version / Commit: PR head 2321c0dbb53b3dbfed1fef52ef12e57f1226b7b0 against current main e3cb4a215d76e5df929f60e71552531408f95c72. Runtime measured at b5c443c9310f2118289500b6088822e7bba1fb72 against merged PR80 9adb0701132d259a9fcc8a559472f1c3dc5590b0; all Rust sources, Cargo manifests/lockfile and CUDA build inputs are identical between measured and final heads. Upstream docs/recipes were integrated afterward.

  • cargo fmt --all --check
  • cargo clippy --workspace --locked --all-targets -j 16 -- -D warnings
  • cargo test --workspace --locked -j 16
  • cargo build --workspace --release --locked -j 16
  • mkdocs build --strict, changed-link checks and skill validation.
  • python benchmarks/bench.py validate benchmarks/smoke.jsonl
  • python -m unittest discover -s tests/benchmarks -p 'test_*.py' -v
  • Controlled default-limit A/B: one excluded feasibility pass per configuration, then two 64-request measured passes at concurrency 1/8/16; real readiness, first inference, warmups, near-limit and expected errors checked separately.
  • Candidate-only limit-one functional check: one burst of eight concurrent 16,368-token requests per model, followed by a recovery request; excluded from performance comparisons.

Test Result

GitHub CI passed on the integrated head: Rust, benchmark tests and Docs build (deployment skipped for pull requests). All local checks passed: 59 workspace tests, nine existing opt-in/resource-dependent cases ignored, eight benchmark tests, strict docs, 96 links and all four updated skills. Scheduler coverage checks full-capacity rejection without dispatch, FIFO order, queued cancellation returning capacity, dispatched cancellation retaining capacity/resources, error/panic release, independent loaded executors and recovery. Native tests cover startup configuration and overload versus ordinary inference-error HTTP bodies.

Fresh GPU A/B: 1,536 measured requests and 256 excluded feasibility, zero measured failures, exact response/key-order parity except Jev timing metadata, and all six regression gates passed. In each limit-one burst, one request returned 200 and seven returned the expected 503; recovery returned 200 with exact parity and healthy readiness. All 40 task processes exited and the reservation was released.

Model Concurrency Baseline mean ms Candidate mean ms Latency change Throughput change p95 change Observed gate
cua 1 23.47 23.53 +0.27% -0.27% +0.39% pass
cua 8 167.52 167.81 +0.17% -0.19% +0.09% pass
cua 16 320.86 321.56 +0.22% -0.25% +0.43% pass
jev 1 108.68 108.69 +0.00% -0.00% -0.77% pass
jev 8 812.37 812.04 -0.04% +0.04% +3.42% pass
jev 16 1515.66 1516.07 +0.03% -0.02% -0.08% pass

Gates were predeclared: mean of per-run mean latency ≤105% baseline, mean requests/s ≥95%, mean per-run p95 ≤110%. These are observed results from two repetitions per setting, not statistically proven equivalence or a speed claim. Metal, CUDA Graphs and full frontend performance were not measured. The nine ignored default-suite cases (six CUDA kernel references, two Laya checkpoint cases and one pinned Jev tokenizer case) were not run separately; no kernel/tokenizer changes are included.

Demo / evidence

Actual-run report, frozen protocol and source/input hashes, and compressed raw records. The report contains both repetitions' variation, process-to-readiness and first-inference results, and memory sampled every 200ms, including near-limit validation. The archive includes all measured/feasibility responses, warmups, first/near-limit/error cases, overload/recovery records, byte-exact synthetic inputs and six raw device-memory sample logs.

Controls: exact reserved GPU5 (SM90 NVIDIA L20X), NUMA1/CPUs56–71, driver 570.133.20, CUDA driver API13000, Rust1.98.1, pinned Cua-S1/Open-Jev checkpoints, BF16 backbone with existing FP32 CPU heads/f64 normalization, unchanged CUDA ABI4 library, eager CUDA and 16 Tokio workers. Models, precision, source/library hashes, prepared-environment requirements, input shapes/lengths, budget and reproduction commands are in the linked protocol/report. Startup, warmups and preparation are excluded from measured replay. Baseline binaries reuse an isolated build whose entire tree equals merged main; measurements of both configurations are fresh.

This is synthetic regression evidence, not model accuracy or a general serving-capacity benchmark. Video and labelled accuracy are N/A for this backend admission change. Public metadata removes machine-local identity/paths/process and reservation details; original response bodies and timing records are retained. No private/customer input or third-party visual assets are included. Full private originals and the launcher remain preserved locally.

Self-review

Agent-assisted precheck found no actionable issues in the complete diff and verified the final head's build-source equivalence to the measured candidate. Contributor and maintainer review remain pending.

  • I have reviewed the full diff and addressed the issues I found.
  • I have checked that the change follows the project's architecture and stays focused on the stated purpose.
  • I have run the checks appropriate to this change and reported commands, results, and anything I could not verify above.
  • I have checked that the PR description, documentation, and any accuracy or performance claims match the implementation and available evidence.

Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
…admission

Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
Signed-off-by: Hongsheng Liu <liuhongsheng4@huawei.com>
@psv666

psv666 commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

CPU review at 2321c0dbb53b3dbfed1fef52ef12e57f1226b7b0: no new runtime correctness finding in the inspected admission/worker changes, with one artifact-hygiene suggestion.

[P3] Keep the raw measurement archive as a durable external evidence asset. docs/benchmarks/native-admission/raw-results.json.gz adds 352,885 bytes of historical run responses, warmups, memory samples and summaries. The repository consumers I found are the accompanying report and its recomputation snippet; no runtime path or maintained regression test consumes it. Under the current committed-artifact rules, please move this one-off run archive to a durable release/evidence asset and update the documentation links. Preserve the complete raw bytes, checksums and recomputation instructions, and retain any small inputs needed as maintained replay fixtures. This is an evidence-storage suggestion, not a reason to delete the measurements.

Personally run: cargo test --locked -p omni-runtime (7 passed), the two workers' HTTP/startup admission checks (8 passed), cargo fmt --all --check, and strict documentation build; all passed. I inspected reservation ownership/release, rejection/status behavior and startup configuration. I did not rerun the historical CUDA/model A/B campaign, so these CPU results do not re-certify its latency, quality or device-memory claims. The PR remains a draft and currently has merge conflicts; validation here covers its own head.

Review scope and provenance
  • Reviewed head: 2321c0dbb53b3dbfed1fef52ef12e57f1226b7b0; checked unchanged immediately before publication.
  • PR API base (main): e3cb4a215d76e5df929f60e71552531408f95c72; diff merge-base: e3cb4a215d76e5df929f60e71552531408f95c72. Validation above used an isolated archive of the reviewed head.
  • Authored code/test diff: +227/−23; total text diff: +736/−43 (generated fixtures/docs counted only in total).
  • Repository InferMatrixLab/system1-omni (ID 1386454853). Applied .agents/skills/system1-omni-review/SKILL.md from trusted main@5ba6a3ba668cb33028eb0bce2c6d17cdc53aebe7, SHA-256 ae2dfac116376ae2ec41f65122cc1bf5aea10751b8b907198a56154e5d67230e. Loaded references: repository contracts, architecture, CONTRIBUTING, self-review artifact/demo rules, CI/docs workflows and the affected component documentation.

psv666 commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

The next integration step has changed since the earlier review: #149 has landed, and current runtime already provides BoundedAdmission, owned request/cost permits and ExecutionContext. Decider opts in; Cua/Open-Jev retain their existing admission behavior.

Before resolving this draft’s conflicts, please narrow the delta to the Cua/Open-Jev policy and integration that remain needed. State precisely whether “pending” counts complete requests or model execution units (Cua admits one question, Open-Jev a complete request), and reuse the landed runtime primitives where their semantics fit. Preserve capacity ownership through detached blocking work after timeout/disconnect, and cover full-capacity refusal, cancellation and permit release in the resolved tree.

The earlier archive-hygiene suggestion also remains: retain the raw measurements/checksums/recomputation path as durable evidence. Keep historical GPU A/B results pinned to their measured head; describe the executable delta after integration and rerun the impacted admission/latency measurements if that behavior changes. A storage-only move does not itself require repeating the GPU campaign.

Please update the self-review evidence and draft readiness after current-head CI/CPU coverage is available. This follow-up is at unchanged head 2321c0dbb53b3dbfed1fef52ef12e57f1226b7b0; the prior test results are historical, not newly rerun here.

Agent-assisted readiness follow-up using the repository review skill pinned at trusted main cd302b6e3d7723ebfc33bba9698da2c1e881f203 (SHA-256 ae2dfac116376ae2ec41f65122cc1bf5aea10751b8b907198a56154e5d67230e). This comment reconciles current main and the existing discussion; it is not a new full code-review approval or a claim of newly executed model tests.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants