Repository navigation
Docs: replace an unreproducible dsv4 bind-phase baseline - #2034
Merged
ChaoWao merged 1 commit intoAug 26, 2026
Merged
Conversation
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The paragraph correcting dsv4's `args` and `host_view_close` rows cited measurements taken on `f830f13c3` plus a then-uncommitted child-memory change, so its baseline cannot be reproduced from any commit in the tree. Its `host_view_close` figure was also measured before hw-native-sys#1973 removed the `halHostRegister` side, leaving it an order of magnitude high: a reader A/B-ing against 0.014-0.028 ms today would read a 10x regression where there is none. Both figures now come from the merged tree at `dcf7559e8`, 12 binds over two ranks at `--rounds 6`: `args` 0.036-0.075 ms, `host_view_close` 0.0012-0.0030 ms with `count=0 bytes=0`. hw-native-sys#2022 established that giving a phase's counters the span its clock covers changes what the numbers mean and not their values, so those remain the current figures. The same run's peak host RSS is recorded beside them, since the rows they correct are per-byte costs over what a bind stages: 1.31 GiB across the whole process tree under `--skip-golden`, 23.4 GiB when the fixture is streamed in, against the ~45.5 GB per rank the pinned row cost. The pinned table itself is untouched. It is anchored to `777d4171` on purpose and says so.
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
docs/dfx/hbg-bind-phases.mdcorrects two rows of its pinned table — dsv4'sargsandhost_view_close— in a paragraph that tells the reader what toexpect on a current tree. Both of its figures were unusable for that:
f830f13c3plus a then-uncommitted child-memory change,so it cannot be reproduced from any commit in the tree.
host_view_closewas measured before Fix: prefer staging views for host tensor access #1973 removed thehalHostRegisterside, leaving it an order of magnitude high. A reader A/B-ing against
0.014–0.028 ms today reads a 10x regression where there is none.
Both now come from the merged tree at
dcf7559e8, 12 binds over two ranks at--rounds 6:args0.036–0.075 ms,host_view_close0.0012–0.0030 ms withcount=0 bytes=0. #2022 established that giving a phase's counters the span itsclock covers changes what the numbers mean and not their values, so these remain
the current figures and were not re-measured on top of it.
Peak host RSS is recorded beside them, because the rows being corrected are
per-byte costs over what a bind stages: 1.31 GiB across the whole process
tree under
--skip-goldenand 23.4 GiB when the fixture is streamed in,against the ~45.5 GB per rank the pinned row cost. That also retires the local
note that dsv4 was SIGKILLed at
--rounds 6— both modes now complete sixrounds with exit 0.
The pinned table itself is untouched. It is anchored to
777d4171onpurpose and says so ("For orientation, not thresholds … Re-measure rather than
trusting this table"), so refreshing its rows would work against its intent.
Only the paragraph that speaks for the current tree is changed.
Testing
markdownlint-cli2 --config tests/lint/.markdownlint.yaml— 0 errorstests/lint/check_retired_names.py— cleandcf7559e8via the doc's own recipe, two arms,task-submit --device auto --device-num 2, 12 binds eachDocs-only; no code path is touched.