Skip to content

laya: add a reproduction command for every Apple Silicon benchmark number - #67

Open
cacheline999 wants to merge 12 commits into
ThinkFlowLab:mainfrom
cacheline999:laya-review-followups
Open

cacheline999 wants to merge 12 commits into
ThinkFlowLab:mainfrom
cacheline999:laya-review-followups

Conversation

@cacheline999

@cacheline999 cacheline999 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Follow-ups to #30:

  1. Every performance number in the Apple Silicon recipe now has a command that reproduces it. Laya on Apple Silicon: worker, benchmarks and recipe #30
    shipped scripts for the main comparisons, but several numbers in the recipe came from one-off probes
    that were not in the repository: requests after an idle gap, the cost of new input lengths and the
    memory they add, a checkpoint loaded while serving, and the CPU fallback. The benchmark README now
    has a table from each recipe claim to the command behind it, and the missing pieces are added:

    • paired.py --gap SECONDS (each request after that much idle, on a fresh connection) and --only;
    • lengths.py: first request of new input lengths and the footprint they add, for one or two flag sets;
    • late_load.py: a checkpoint loaded while serving, its first request directly and through the frontend;
    • fallback.py: Laya's fallback to the CPU under a lowered MPS memory limit;
    • release.py: the memory torch.mps.empty_cache() gives back after many new lengths, and whether
      those lengths are cold again afterwards. It prepares the model with the worker's own startup
      (build_app: options, warmup, exit if not on MPS) and walks lengths with lengths.py's walk;
    • a loop of fresh starts for the first request after ready, alternating the worker with plain
      laya-serve.

    late_load.py, fallback.py and release.py refuse a measured run on battery or under load like
    the other scripts (--feasibility runs anyway); the check and the workload reader are now shared
    helpers in env.py.

    Two statements had no command behind them and were changed instead: the recipe no longer gives
    numbers for keeping the GPU busy between requests (it only says the worker does not do this), and
    the one 71 s late load is now attributed to what it was, a second worker holding memory on the same
    GPU, rather than to load in general. The recipe's count of every length up to the window is now the
    454 that the full-window command reaches (it said 477, from an earlier probe with shorter inputs).

    Each is an A/B where the claim is a comparison (with the options against without, or a worker against
    plain laya-serve). A rerun gives other numbers on another Mac or under other load; the comparison is
    what should carry over. Rerunning everything on the M1 Pro moved two numbers in the recipe, both now
    given as ranges (see Test Result).

  2. Readiness contract test (flaky on an M4 in the last review): it now checks that the first request
    after ready succeeds with a valid answer, without a latency bound. That latency depends on how long
    the GPU has been idle and on the Mac (an M5 answered a first request without any warmup in 132–143 ms),
    so it stays in the benchmarks. That the warmup ran before ready is still tested.

  3. ruff format drift: the Laya files are formatted with ruff 0.16.10 defaults (88 columns, as the rest
    of the repository; the first commit is formatting only, with every file's syntax tree unchanged), and
    requirements-mps.txt pins ruff, pytest and httpx2. Comments that the formatter had pushed onto
    closing brackets are back above their statements, so the two noqa: SIM115 suppress again.

  4. Stale docstring in tests/laya/test_worker.py; the reviewer's M4 is listed in the recipe.

No change to the worker's runtime code. A CI job for tests/laya was suggested in the review; happy to
add one in a separate PR if you want it.

Test Plan

System1-Omni Version / Commit: 8a5ff37 on top of 58b8cbe

  • Every new reproduction command run once on the M1 Pro (commands as listed in the benchmark README).
  • ruff format --check and ruff check --select E4,E7,E9,F on the Laya files, ruff 0.16.10.
  • PYTHONPATH=src python -m pytest tests/laya, and with LAYA_CONTRACT=1; contract tests on MPS
    without and with --compile --weights fp16.
  • The documented-commands test now also checks the inline commands of the reproduction table against
    each script's flags (a misspelled flag fails it).
  • mkdocs build --strict.

Test Result

Reruns on the M1 Pro (16 GB), on battery with a 1-minute load of 9–17 (above 100 during the last two steps; other applications), so the
absolute numbers are high and noisy; what matters is that each command runs and shows the effect the
recipe describes:

claim recipe rerun
request after 2 s idle, options / none ~0.9 (105–115 vs 114–127 ms) 0.89 (950 vs 1020 ms, 30 pairs, wide interval)
first request of a new length, extra 15 ms with the options, 6 without 28 and 13 ms (25 and 8 ms over all 454)
footprint per 100 new lengths +520 MB with the options, +64 without +539 and +72 MB
footprint after every length up to the window 5.3 GB with the options, 4.0 GB without, not growing 5.2 GB (from 2.8) and 3.7 GB (from 3.8) over 454 lengths → recipe now says 5.2 and 3.7 GB after 454
checkpoint loaded while serving, first request 5–10 s without, ~70 s with --compile 6.5 s and 19–22 s (200 through the frontend) → recipe now says 19–22 s, and 71 s once with a second worker on the GPU
CPU fallback, the request that ran out of memory ~30 s 73 s plain, 68 s with the options, 33 s on a second plain run → recipe now says 30–73 s
CPU requests after the fallback 140–270 ms 158–273 ms (2.0 s for the first)
empty_cache() after 100 new lengths, with the options (release.py --compile --weights fp16) releases most of it; those lengths pay again after a release 2946–2953 MB in three runs (start 2942–3016, after the lengths 3231–3481), 2896–2897 after running them again and a second release; extra on a length 16–18 ms before and after → recipe now says it comes back to about 2.95 GB
first request after ready, one fresh start each — worker with the options 75 ms (38 s to ready), plain laya-serve 227 ms (6.5 s); the loop runs 23 of each
checkpoint download 97 s 401 s (network)

A self-review of the first version of this PR found that lengths.py skipped lengths by word count, which
did not match what the warmup ran, and that the README's full-window command repeated the 512-token
length from the 111th step on. It now reads the warmup's token lengths back from the workers, skips them,
and stops at the window: over every length the rerun measured 454 distinct lengths from 57 to 512 tokens,
leaving out exactly the warmup's 182 and 432. In the same pass: late_load.py waits up to --timeout
(the client's 120 s could cut off a first run that also downloads), fallback.py reuses the shared
start/stop helpers, and the README no longer says paired runs need no idle machine (every script refuses a
measured run on battery or under load; a rerun here was refused). A second pass: late_load.py sends the
next request only when the late checkpoint became resident, so a load cut off by the frontend's 504 or the
client's timeout is still followed up, and a failed (unloaded) one is not loaded a second time; its
docstring explains every field it prints, and it rejects a --late that names the startup checkpoint; lengths.py records whether the walk reached the window or the count asked for, and the
late-load timeout test now checks the client the late request uses (the earlier one passed with the
timeout removed).

Checks: ruff format and check clean; 98 unit tests passed, 118 with LAYA_CONTRACT=1; contract on MPS
20/20 plain and with the options; strict docs build passes.

A third self-review pass found that release.py loaded the model its own way (no device check, its own
warmup, W1 taken as the file's first line, warm lengths and the window not handled), that three new
scripts skipped the noise check the README promised, that the fresh-start loop had no laya-serve side,
and that fallback.py hid a failed memory probe behind a ValueError. Each has a test that failed
before the fix. Every changed script was rerun once on MPS (late load 8.5 s plain, fallback 31 s and
then CPU, lengths, paired, bench_http on both sides, bench_inproc, profile_mps); the load was 5–9, so
those were --feasibility runs, and the three scripts refused without it.

A fourth and fifth pass corrected the release measurement. An intermediate version of release.py
started from whatever the warmup had left cached, which varied by hundreds of MB between runs (2761 to
3349 MB), so the share it gave back looked like half in one run and all of it in another; for one commit
this was wrongly credited to a torch.mps.synchronize() before the release. An A/B in one process
showed no effect of the synchronize (it waits 0.004 ms after a request; fresh processes with and without
it end within 11 MB), so it is gone. release.py now releases once before the walk, so both reruns
started at 2771 MB, and releases a second time after running the lengths again. A sixth pass found
that the footprint shows a release up to about two seconds late, and that even after a release the
starting footprint varies by a few hundred MB between processes, so no share of "what the lengths
added" is stable. release.py now reads each footprint --settle seconds (default 3) after a release,
and the recipe gives where a release lands (about 2.95 GB in three runs) instead of a percentage. In the same passes:
release.py refuses in one line off MPS and keeps the traceback of any other failure, late_load.py
and fallback.py record the noise in their output under --feasibility, a repeated workload id is
refused, and lengths.py reads the warm lengths by replaying engine.warmup. The release numbers above
are --feasibility runs at load 6–40; they report memory, which the load moves far less than latency,
and the two runs agree within 12 MB.

Self-review

  • 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.

…0 defaults

Formatting only (88 columns, as the rest of the repository); no behaviour change.
… M4 in the recipe

- The contract test of the first request after ready asserts that it succeeds with a valid answer. Its
  latency depends on how long the GPU has been idle and on the Mac (an M5 answered a first request without
  warmup in 132-143 ms), so it is left to the benchmarks; that the warmup ran before ready is tested
  separately.
- requirements-mps.txt pins pytest, httpx2 and ruff; the recipe's Test section runs the ruff checks.
- The recipe lists the reviewer's M4 among the Macs the tests ran on.
- Test docstring points at tests/laya.
…cipe

The benchmark README now maps each claim in the recipe to the command that produces it.
New: paired.py --gap (requests after an idle pause, on fresh connections) and --only; lengths.py (first
request of new input lengths and the memory they add, for one or two flag sets); late_load.py (a checkpoint
loaded while serving, directly and through the frontend); fallback.py (Laya's CPU fallback under a lowered
MPS memory limit). A rerun of each on the M1 Pro moved two numbers in the recipe: the late load with
--compile took 19-22 s (71 s once under heavy load), and the out-of-memory request 30-73 s.
… the window; review fixes

- lengths.py read the warmup's token lengths back from the workers and skips them; the walk stops where
  longer states are truncated to the window. Before, the skip compared word counts with another state
  and question, and --lengths 477 repeated the 512-token length from the 111th step on.
- late_load.py waits up to --timeout (default 900 s) for the late request; the client's 120 s cut off a
  first run that also downloads the checkpoint.
- fallback.py starts and stops the worker with paired.spawn/stop (spawn takes extra environment).
- README: every script refuses a measured run on battery or under load (paired runs too); the index has
  fp16 weights alone, and the full-window lengths command.
…records why it stopped

- late_load.py: when the late request fails, it does not send the next one, which would load the
  checkpoint again and report that as a request's latency; `next` is null.
- lengths.py records whether the walk reached the window or the number of lengths asked for, and the
  summary says which.
- The late-load timeout test now checks the timeout the late request's client gets; the old one passed
  with the timeout removed.
…that failed

Whether to send the next request now depends on whether the late checkpoint became resident, not on the
first request's status: a load that the frontend cut off with 504, or that outlasted the client, still
succeeds in the worker, and its next request is the measurement the recipe cites. A failed load is
unloaded, so it is still not followed by a second load. The output reports `loaded`.
@cacheline999 cacheline999 changed the title Laya: follow-ups from the #30 review laya: add a reproduction command for every Apple Silicon benchmark number Oct 3, 2026
…nt already served

- The docstring describes every field late_load.py prints, including `loaded` and why `next` can be null;
  a test keeps the two in step.
- --late naming the startup checkpoint, or an alias of it, is rejected: the request would hit the warm
  checkpoint and read as a failed load.
…reproduced numbers

release.py measures, in process, the memory torch.mps.empty_cache() gives back after
100 new input lengths and whether those lengths are cold again afterwards. The
benchmark README now indexes it, and a loop of fresh starts for the first request
after ready.

The recipe no longer quotes heartbeat numbers that no script reproduces, and the
71 s late load is attributed to what it was: a second worker holding memory on the
same GPU.
…y runs the worker's own startup

- late_load.py, fallback.py and release.py now refuse to measure on battery or above --max-load
  unless given --feasibility, as the README already said of every script; the check and the
  workload reader are shared helpers in env.py.
- release.py prepares the model with the worker's build_app (options, warmup, and an exit off
  MPS) and walks lengths with lengths.measure, so the warmup's lengths are skipped and the walk
  stops at the window. Through that path empty_cache() gives back about half of the memory the
  lengths added, not most of it; the recipe now says so.
- release.py looks W1 up by id; fallback.py reports why the MPS memory probe failed.
- The fresh-start loop alternates the worker with plain laya-serve, the comparison the recipe makes.
- Comments that ruff format had pushed onto closing brackets are back above their statements, and
  the two noqa: SIM115 comments are on the lines they suppress again (syntax trees unchanged).
…feasibility result

- release.py synchronizes before torch.mps.empty_cache(). Without it the release ran while the
  last requests could still hold their buffers and gave back about half; with it the footprint
  returns to 3.0 GB after 100 new lengths. The recipe says so again, and its length count is the
  454 the full-window command reaches rather than 477 from an earlier probe.
- release.py refuses in one line when the model is not on MPS.
- late_load.py and fallback.py record the noise in their output when run with --feasibility.
- read_workloads refuses a repeated id instead of dropping one of them.
- warmup_lengths replays engine.warmup instead of rebuilding its requests.
- RUSAGE_INFO_V4 is a named constant; the noise test no longer patches laya.load.
…; no synchronize

The previous commit added torch.mps.synchronize() before empty_cache() and credited it with
the difference between giving back half and giving back everything. An A/B in one process does
not support that: a synchronize right after a request waits 0.004 ms, and fresh processes
releasing with and without it end within 11 MB of each other. What differed between those runs
was the starting footprint, which included whatever the warmup had left cached (2761 to 3349 MB).

release.py now releases once before the walk, so runs start from the same footprint (2771 MB in
both reruns), and releases a second time after running the lengths again. With the options it
gives back about 60% of what 100 new lengths added, in both reruns; the recipe says so. The
synchronize and its test are gone, and a failure other than the device check keeps its traceback.
…; recipe gives where it lands

macOS shows a release in the process footprint up to about two seconds later (without the options
the baseline release still read 4180 MB at once and 3154 MB a second later). release.py now waits
--settle seconds (default 3) after each release before reading, and records the wait.

The footprint at the start still varies between runs by a few hundred MB even after a release
(2942 to 3016 MB in three runs, 2939 and 3319 MB in a trace), so a share of what the lengths added
is not a stable number. Where a release lands is: 2946 to 2953 MB in three runs, 2896 to 2897 MB
after running the lengths again. The recipe now says the footprint comes back to about 2.95 GB,
near where the worker starts, instead of a percentage from a second baseline.

prepare() checks the device itself, so build_app no longer checks it again; the duplicate test of
that refusal is gone.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant