Repository navigation
perf(householder): no parallel region below the compact factor's own floor - #9
Merged
Merged
Conversation
The row the patch adds below the experiments table: what the floor is, the measured before/after, and that the tall shapes are untouched.
…floor `upgrade_householder_factor` threads its `V^H V` products, and its `deep` predicate already states when that reduction has the shape to pay for it: deeper than twice its `n x n` output, at least one row per thread, over the `8 * 1024` work floor. Below that floor the products still opened a rayon region each, and a blocked factorisation calls this function once per block -- at a block size the recursion halves every level. A threaded 128x128 QR therefore paid the fork-join cost of `n`-wide reductions about fifty times for work the same floor calls too small to split, and measured 4.2-4.6x *slower* than the sequential call. Asking the floor once, before opening anything, is the whole change: below it the call takes `Par::Seq`, above it the caller's `par` is forwarded exactly as before. The predicate is the one the products below already use, so no new constant and no change to what is computed. Isolated driver on the call tlinalg's `qr` family makes (`qr_in_place`, CPUs 4-11, best of 7, f64 unless stated), ms at Par::rayon(8), before -> after: | shape | before | after | before / seq | after / seq | |---|---|---|---|---| | 128x128 | 1.05 | 0.235 | 4.2-4.6x | 1.00 | | 192x192 | 1.79 | 0.567 | 2.9-3.0x | 0.94 | | 256x256 | 2.46 | 0.917 | 2.1-2.3x | 0.79 | | 384x384 | 3.12 | 1.85 (c64 512: 7.36 -> 5.18) | 1.3-1.4x | 0.60 | | 512x512 | 4.59 | 2.85 | 0.96-0.98 | 0.44 | | 768x768 | 7.32 | 5.47 | 0.49 | 0.38 | | 1024x64 | 0.65 | 0.466 | 1.34-1.44x | 1.00 | | 4096x256 | 6.35 | 6.31 | 0.40 | 0.40 | Every shape improves or is unchanged: the tall shapes the ledger's other rows serve keep their parallelism, because their reductions are above the floor, and below it no region is opened at all rather than a smaller one. Gate: `cargo test -p t4a-faer --lib` and `--doc`; one pre-existing failure in the fork is unrelated to this change (`sparse::linalg::qr::tests:: test_numeric_simplicial_qr_1_transpose` fails the same way on the parent commit).
This was referenced Oct 14, 2026
shinaoka
added a commit
to tensor4all/tlinalg-rs
that referenced
this pull request
Oct 14, 2026
tensor4all/faer#9 merged as 04542fae, so the region floor comes from the fork's main. The tree is the same one the pair recorded at 7fdba8a named, so the pages stand.
shinaoka
added a commit
to tensor4all/tlinalg-rs
that referenced
this pull request
Oct 14, 2026
tensor4all/faer#9 merged as 04542fae, so the region floor comes from the fork's main. The tree is the one the recorded pair named, so the pages stand.
shinaoka
added a commit
that referenced
this pull request
Oct 14, 2026
The fork's whole purpose is a measurable delta against upstream, so the reason for each ledger row and the course of an optimisation have to survive the session that made it. AGENTS.md now points at the shared Work Logs And Design Records policy and the local format in docs/worklogs/, states that upstream feedback is deferred at this stage, and says where an unmeasured lead belongs. The first record covers #9: the floor the compact factor now asks once, the two candidates that lost with the measurements that killed them, how the fork and upstream compare, and the leads this session did not measure.
shinaoka
added a commit
that referenced
this pull request
Oct 14, 2026
The fork's whole purpose is a measurable delta against upstream, so the reason for each ledger row and the course of an optimisation have to survive the session that made it. AGENTS.md now points at the shared Work Logs And Design Records policy and the local format in docs/worklogs/, states that upstream feedback is deferred at this stage, and says where an unmeasured lead belongs. The first record covers #9: the floor the compact factor now asks once, the two candidates that lost with the measurements that killed them, how the fork and upstream compare, and the leads this session did not measure.
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.
What
upgrade_householder_factoropens a rayon region for itsV^H Vproducts whenever the caller asks for threads — and its owndeeppredicate right beside them already states when that reduction has the shape to pay for it (deeper than2 * n, at least one row per thread, over the8 * 1024work floor). Below that floor the products still opened a region each, and a blocked QR calls this function once per block, at a block size the recursion halves every level. A threaded128 x 128QR thus paid the fork-join cost ofn-wide reductions about fifty times for work the same floor calls too small to split.The change asks the floor once, before anything is opened: below it the call takes
Par::Seq, above it the caller'sparis forwarded exactly as before. Nothing about what is computed changes; the predicate is the one the products themselves already use, so no new constant.Measured (isolated driver on the call tlinalg's
qrfamily makes:qr_in_place, CPUs 4-11, best of 7)ms at
Par::rayon(8), before → after (f64 unless stated):Every shape improves or is unchanged. The tall shapes the ledger's other rows serve keep their parallelism (their reductions are above the floor); below the floor no region is opened at all rather than a smaller one.
Two neighbouring candidates were measured and dropped for this PR, both recorded here so they are not re-tried: gating the per-block apply on the trailing area (
(m - row) * (n - col) < par_threshold) is harmful on tall panels — their block updates arerows x local x a1colswith a long depth, so an area threshold sends real wins toPar::Seq(4096x256 f64 6.36 → 6.82, c64 16.53 → 18.23) — and forcing then x n x nhead product of theprev_block_size < 8branch toPar::Seqcosts 38% at 512x512 f64 (2.49 → 3.43).Gate
cargo test -p t4a-faer --lib(202 passed) and--doc(37 passed). One pre-existing failure in the fork is unrelated to this change:sparse::linalg::qr::tests::test_numeric_simplicial_qr_1_transposepanics on an assertion intriangular.rs:990on the parent commit too (verified on a clean checkout of22f23cf).PATCHSET.mdcarries the row, including the measurement protocol.