Layout time doubles per nesting level: `computeMinContentMainSize` recurses twice per child
I maintainer di solito rispondono entro 1 giorno
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 68/100
- Tipo di issue
- Bug
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Tranquilla
- Stack tecnologico
- cpp
- Ambito
- performance
Direzione di ricerca
Inizia in yoga/algorithm/CalculateLayout.cpp, in computeMinContentMainSize e nel suo chiamante, computeAutoMinMainSize. Confronta il comportamento di layout esistente con la traversata combinata proposta, quindi verifica che i layout profondamente annidati scalino linearmente e che la geometria rimanga invariata usando l’harness differenziale descritto nell’issue.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
computeMinContentMainSize in yoga/algorithm/CalculateLayout.cpp is a
per-axis function, so the container branch recurses into every child twice
-- once for the node's main axis and once for its cross axis:
float childMain = computeMinContentMainSize(
child, nodeMainAxis, direction, ownerWidth, ownerHeight);
...
float childCross = computeMinContentMainSize(
child, nodeCrossAxis, direction, ownerWidth, ownerHeight);
Each call re-walks that child's entire subtree and keeps a single component,
so a chain of flex containers costs T(d) = 2*T(d-1) -- i.e. O(2^depth).
Nothing memoizes it: the recursion does no layout writes and does not go
through the layout cache, so the cost is invisible to the cache counters.
This is reachable from ordinary markup. computeAutoMinMainSize calls it for
every in-flow flex item whenever the config does not set
Errata::MinSizeUndefinedInsteadOfAuto -- i.e. under the modern default --
which is the CSS Flexbox §4.5 automatic minimum size path.
Repro
Plain nested flex rows, no measure functions, no custom config beyond
YGErrataNone, one concrete leaf at the bottom:
#include <yoga/Yoga.h>
#include <chrono>
#include <cstdio>
#include <vector>
static double timeDepth(int depth) {
YGConfigRef config = YGConfigNew();
YGConfigSetErrata(config, YGErrataNone);
std::vector<YGNodeRef> chain;
YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetWidth(root, 800);
YGNodeStyleSetHeight(root, 600);
chain.push_back(root);
for (int i = 0; i < depth; i++) {
YGNodeRef n = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexDirection(n, YGFlexDirectionRow);
YGNodeInsertChild(chain.back(), n, 0);
chain.push_back(n);
}
YGNodeRef leaf = YGNodeNewWithConfig(config);
YGNodeStyleSetWidth(leaf, 20);
YGNodeStyleSetHeight(leaf, 20);
YGNodeInsertChild(chain.back(), leaf, 0);
auto t0 = std::chrono::steady_clock::now();
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
auto t1 = std::chrono::steady_clock::now();
YGNodeFreeRecursive(root);
YGConfigFree(config);
return std::chrono::duration<double>(t1 - t0).count();
}
int main() {
for (int d : {16, 18, 20, 22, 24, 26}) {
std::printf("%3d %8.3f s\n", d, timeDepth(d));
std::fflush(stdout);
}
}
main@433d463, clang -O2 (Apple M-series), median of 5 runs:
| depth | layout time | vs previous shown |
|---|---|---|
| 16 | 0.004 s | |
| 18 | 0.017 s | 4.2x |
| 20 | 0.065 s | 3.8x |
| 22 | 0.264 s | 4.1x |
| 24 | 1.055 s | 4.0x |
| 26 | 4.233 s | 4.0x |
Steady ~4x per two levels, i.e. ~2x per level -- the 2^depth signature.
Extrapolating the same factor, depth 30 lands around a minute, which reads as
a hang rather than as slowness.
Proposed fix
Compute both components in a single walk. Each subtree then costs exactly one
recursion and the per-pass cost goes back to linear.
Semantics are preserved point for point:
- static per-axis values still win, and when both are defined the function
still returns without recursing (matching today's early return); - leaves still measure once per axis and still add their own padding/border;
- each child's contribution is projected onto the parent's main/cross axes,
which is exactly what the two separate calls produced.
The shape of the change, condensed (full patch in a PR if you want it):
-static float computeMinContentMainSize(
- yoga::Node* const node,
- const FlexDirection requestedAxis,
- const Direction ownerDirection,
- const float ownerWidth,
- const float ownerHeight) {
- const bool wantRow = isRow(requestedAxis);
- const FloatOptional staticMin =
- wantRow ? node->getMinContentWidth() : node->getMinContentHeight();
- if (staticMin.isDefined()) {
- return staticMin.unwrap();
- }
+struct MinContentSize {
+ float width;
+ float height;
+};
+
+static MinContentSize computeMinContentSize(
+ yoga::Node* const node,
+ const Direction ownerDirection,
+ const float ownerWidth,
+ const float ownerHeight) {
+ const FloatOptional staticW = node->getMinContentWidth();
+ const FloatOptional staticH = node->getMinContentHeight();
+ // Both pinned -> still no recursion at all, as before.
+ if (staticW.isDefined() && staticH.isDefined()) {
+ return {staticW.unwrap(), staticH.unwrap()};
+ }
// ... measure-func branch: measure once per axis, add own padding/border
// (unchanged, just returning both components)
for (size_t i = 0; i < node->getChildCount(); i++) {
...
- float childMain = computeMinContentMainSize(
- child, nodeMainAxis, direction, ownerWidth, ownerHeight);
+ // One recursion per child, then project onto this node's axes.
+ const MinContentSize childSize =
+ computeMinContentSize(child, direction, ownerWidth, ownerHeight);
+
+ float childMain = nodeMainIsRow ? childSize.width : childSize.height;
childMain += child->style().computeMarginForAxis(nodeMainAxis, ownerWidth);
-
- float childCross = computeMinContentMainSize(
- child, nodeCrossAxis, direction, ownerWidth, ownerHeight);
+ float childCross = nodeMainIsRow ? childSize.height : childSize.width;
childCross +=
child->style().computeMarginForAxis(nodeCrossAxis, ownerWidth);
mainTotal += childMain;
crossMax = std::max(crossMax, childCross);
}
// ... padding/border added per axis (unchanged)
- return wantRow ? widthMin : heightMin;
+ return {
+ staticW.isDefined() ? staticW.unwrap() : widthMin,
+ staticH.isDefined() ? staticH.unwrap() : heightMin};
}
The single caller in computeAutoMinMainSize then projects instead of
picking an axis up front:
- const FloatOptional contentMain = FloatOptional{computeMinContentMainSize(
- child, mainAxis, direction, ownerWidth, ownerHeight)};
+ const MinContentSize contentSize =
+ computeMinContentSize(child, direction, ownerWidth, ownerHeight);
+ const FloatOptional contentMain =
+ FloatOptional{isRow(mainAxis) ? contentSize.width : contentSize.height};
One subtlety worth calling out: when only one axis has a static
min-content value the function must still recurse for the other, so the
static value is applied per-axis at the return sites rather than as a
single early exit. That matches what the two per-axis calls did (one
short-circuited, the other recursed).
With the patch, same machine and harness:
| depth | 26 | 40 | 60 | 100 | 200 |
|---|---|---|---|---|---|
| layout time | 0.00002 s | 0.0001 s | 0.0001 s | 0.0003 s | 0.0009 s |
Depth 26 goes from 4.2 s to under a tenth of a millisecond, and depth 200 --
far past where stock yoga stops finishing at all -- costs under a millisecond.
Results are unchanged. I ran a differential harness over 300
pseudo-randomly generated trees (mixed row/column, percent and fixed sizes,
flex grow/shrink, min/max constraints, padding/margin/border, aspect-ratio,
wrap, overflow:scroll, absolute children, both LTR and RTL), dumping
left/top/width/height for every node -- 17,094 lines of geometry, byte
identical between stock and patched builds.
Happy to open a PR with the change plus the differential harness as a test if
that shape is useful; let me know if you would rather have it as a benchmark
under benchmark/ or a gtest case.
- Lingua principale
- C++
- Stelle
- 18.9k
- Fork
- 1.6k
- Merge medio
- 1m
- PR unite (30g)
- 1
Preparare l'ambiente
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di react/yoga
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 72/100
react/yoga#2027 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 4/5 3-5 giorni Idoneità per principianti 52/100
I maintainer di solito rispondono entro 1 giorno
-
[Feature] flex-wrap: balanceAperta
Difficoltà 5/5 Più di una settimana Idoneità per principianti 30/100
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 25/100
I maintainer di solito rispondono entro 1 giorno
-
Build tests/YGPersistenceTest.cpp fails with array-bounds error on GCC 16 (-O2 / -O3) due to -WerrorAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 58/100
I maintainer di solito rispondono entro 1 giorno
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
hyprwm/aquamarine#426 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 62/100
amnezia-vpn/amnezia-client#3222 ·
I maintainer di solito rispondono entro 2 giorni
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
valkey-io/valkey-search#1465 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
KhronosGroup/Vulkan-Tutorial#524 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
microsoft/onnxruntime-genai#2633 ·
I maintainer di solito rispondono entro 1 giorno