laya: add a reproduction command for every Apple Silicon benchmark number - #67
Open
cacheline999 wants to merge 12 commits into
Open
cacheline999 wants to merge 12 commits into
cacheline999 wants to merge 12 commits into
Conversation
…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`.
…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
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.
Purpose
Follow-ups to #30:
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 memorytorch.mps.empty_cache()gives back after many new lengths, and whetherthose 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 withlengths.py's walk;laya-serve.
late_load.py,fallback.pyandrelease.pyrefuse a measured run on battery or under load likethe other scripts (
--feasibilityruns anyway); the check and the workload reader are now sharedhelpers 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).
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.
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.txtpins ruff, pytest and httpx2. Comments that the formatter had pushed ontoclosing brackets are back above their statements, so the two
noqa: SIM115suppress again.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/layawas suggested in the review; happy toadd one in a separate PR if you want it.
Test Plan
System1-Omni Version / Commit:
8a5ff37on top of58b8cberuff format --checkandruff check --select E4,E7,E9,Fon the Laya files, ruff 0.16.10.PYTHONPATH=src python -m pytest tests/laya, and withLAYA_CONTRACT=1; contract tests on MPSwithout and with
--compile --weights fp16.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:
--compileempty_cache()after 100 new lengths, with the options (release.py --compile --weights fp16)A self-review of the first version of this PR found that
lengths.pyskipped lengths by word count, whichdid 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.pywaits up to--timeout(the client's 120 s could cut off a first run that also downloads),
fallback.pyreuses the sharedstart/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.pysends thenext 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
--latethat names the startup checkpoint;lengths.pyrecords whether the walk reached the window or the count asked for, and thelate-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 MPS20/20 plain and with the options; strict docs build passes.
A third self-review pass found that
release.pyloaded the model its own way (no device check, its ownwarmup, 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.pyhid a failed memory probe behind aValueError. Each has a test that failedbefore 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
--feasibilityruns, and the three scripts refused without it.A fourth and fifth pass corrected the release measurement. An intermediate version of
release.pystarted 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 processshowed 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.pynow releases once before the walk, so both rerunsstarted 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.pynow reads each footprint--settleseconds (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.pyrefuses in one line off MPS and keeps the traceback of any other failure,late_load.pyand
fallback.pyrecord the noise in their output under--feasibility, a repeated workload id isrefused, and
lengths.pyreads the warm lengths by replayingengine.warmup. The release numbers aboveare
--feasibilityruns 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