Yoga does not respect automatic minimum size for flexbox items #1409
Assignees
DaddyFrosty
Clear assignees
DaddyFrosty
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: DaddyFrosty/yoga#1409
Reference in New Issue
Block a user
No description provided.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description
Actual (all items are equal width):
Expected (1 and 2 should shrink):
React Native Version
0.72.4
Output of
npx react-native infoSteps to reproduce
React Native:
Web equivalent:
Snack, screenshot, or link to a repository
Repro has been provided above.
Stupid bot didn't notice that repro has been provided.
You should provide a link to a repro or a Snack, not a snippet.
Interestingly
react-native-webaddsmin-widthandmin-heightto every view, to avoid overflowing. This behavior is consistent with what Yoga does, but seems to be out of spec.From the CSS spec,
min-widthwas originally 0 in CSS 2 and this is still the case forblockorinlinecontexts but changed toautoin CSS 3. https://www.w3.org/TR/css-sizing-3/#min-size-propertiesFlexbox says this should be the content-based minimum size. So in the correct case, we would be sizing the box according to that, then distribute the rest of the free space accordingly, like the expected example. https://www.w3.org/TR/css-flexbox-1/#min-size-auto
Yoga will incorporate max-content into
flexBasis: 'auto', but doesn't incorporate a minimum size constraintt. From a comment in code:I think this might also depend on https://github.com/facebook/yoga/issues/1298 where right now Yoga doesn't actually have a way to ask for
min-contentin general.Arguably, if we want to build for conformance, this might not be the right choice. But it would maybe be worth seeing how much this trick is actually saving on performance in a real app, if it was an intentional design decision.
FYI @necolas, that we should maybe put
min-width: 0/min-height: 0into the RSD shim unless we decide to fix this. Bu@NickGerleman Huge thanks for looking into this! Do you know of any workarounds?
The workaround for consistency would be set an explicit minimum size, since Yoga will not determine one automatically. That's not really fixing the root issue, admittedly.
I was interested in what the performance impact of implementing "automatic minimum size" might be, so I've done a little informal investigation into the issue.
Methodology
Modifying Taffy's benchmarks, which include head-to-head comparison with Yoga via the rust bindings (yoga-rs) with various style combinations designed to stress the min-content sizing part of the algorithm. In particular, I tested a lot of trees that had
width/heightset toautoso that neither Taffy nor Yoga could easily hit the fast paths that can short-circuit measuring entire subtrees.Findings / Conclusions
Despite the fact that Taffy implements the "automatic minimum size" part of the spec (requested in this issue) while Yoga elides this feature in the name of performance, in practice Taffy is still faster than Yoga for most combinations of styles I tried. The exceptions I found are:
flex-basisis set but notwidth/height(but both Yoga and Taffy are very fast in these cases)flex-direction: columnandflex-wrap: wrapwhich Taffy really doesn't like. However, this case is explicitly called out in the spec as being "insanely expensive" to calculate correctly, and I don't think Taffy is implementing the fast-path exception from the spec for this case. So that's probably unrelated to "automatic minimum size".I should note that this only so consistently true as of a couple of recent performance fixes to Taffy (notably https://github.com/DioxusLabs/taffy/pull/556). And it's entirely possible that such performance wins can be found in Yoga too.
However, if the primary concern is to avoid regressing performance compared to the existing released version(s) of Yoga, then I would be pretty confident that it is possible to implement this feature while maintaining that level of performance, although this may well require some effort to be put into optimising Yoga.
Speaking of which:
Performance of Flexbox implementations seems to be very sensitive to their caching implementations. We've now found tweaks to Taffy's caching logic that give orders of magnitude improvements to certain tree structures on 4 or 5 separate occasions. So that might be a good place to look if we want to improve Yoga's performance (but perhaps it might be better to hold on optimising until a compliant algorithm implementation has been developed so people don't end up depending on perf that isn't achievable within the spec). Yoga and Taffy are both using 9 cache slots, but Yoga uses a FIFO strategy when storing new cache entries, whereas Taffy assigns a slot based on the combination of "measure modes" (to use Yoga's terminology) used when requesting the layout.
Benchmark results
Taffy's standard benchmarks
These are Taffy's current standard benchmarks which we have committed to the repo. The idea of including them here is that they are useful for giving us an insight into what performance in easier / less degenerate cases than the above. Although arguably they measure a bit too easy of a scenario. The use the following style generation function:
ba27f9d])My conclusion from these numbers: although Taffy is a bit faster, the numbers are more similar than they are different (with the exception of the unrealistically large 100k node test).
Stress Test
The results below pertain to trees of nodes where styles were creating using the following function (this is using Taffy/Rust notation, but hopefully it should be pretty clear what it's doing):
these styles were picked for inclusion here because they were amongst the slowest results I could find for both libraries.
ba27f9d])It was interesting (and surprising!) to me that Yoga actually seems to scale worse in this more difficult scenario (note in particular the numbers in the "deep" benchmarks), despite containing algorithm shortcuts designed to facilitate this kind of scalability. Whether that's because the algorithm shortcuts don't help much, or because there are other aspects of Taffy's implementation that compensate for it using a slower algorithm I'm not sure. But I figure that in many ways it doesn't matter too much, because either could be implemented in Yoga.
These benchmarks are definitely a bit artificial, and I'd love to run some numbers on some real-world views/trees, but hopefully they give us some idea of what might be possible :)
Let me know if there are any other style combinations you'd like me to benchmark.
Thanks for the helpful context. Apart from the general "is it possible for this to be efficient", comparing Yoga to previous baseline is probably what we need to do here.
Re benchmarks, I definitely agree with the sentiment of what we have right now veering too far into synthetic. I've been at times tempted to see if we can dump a tree from a real-world app, and make a fixture out of it, so we can test laying out a real-world tree (or different real-world trees), along with incremental mutation and relayout. We do have a couple of ~10k node real-world UIs that have pretty different characteristics.
I've also noticed, at least for the case of
YGBenchmark.c, we sometimes seem to spending more time counting how long it takes to allocate and free Yoga nodes, then the layout process itself. I was experimenting with movingyoga::Styleto a heap allocated structure instead of an inline one, and IIRC the benchmarks showed something like a >10% difference, which was much less visible when isolating to just the layout step (which was slower due to less locality, but a lot less). I'm not aware of whetheryoga-rsdoes much additional boxing on top of Yoga structures, but that might also be a variable to isolate.For caching, beyond what Yoga does, we recently discovered RN Fabric can invalidate the layout cache much more often than it needs to. In RN there is a Yoga node per ShadowNode, and a series of bugs which leads to sometimes restoring previous ShadowNodes, containing pre-layout information, into new trees, on React mutation. In the most serious cases, this could lead to small UI changes having catastrophic effect to layout cache, and effectively removing most of the previous work.
ef438464fawas one of the changes to help here, but there are more coming, and some discussion on the general model (RN Fabric heavily relying on Yoga private APIs is another thing I'd like to get us out of).Apart from cost of flex layout, I think part of the performance concern here is for laying out text.
I think if we wanted to implement this correctly, we could still shortcut min-content measurement in most cases I think.
If flex basis is based on content, we already have a max-content measurement. If we don’t need to shrink, and didn’t otherwise have an explicit flex basis, we know we are already at least min-content sized in the main axis.
Downside, the elegant way to do this involves breaking every Yoga measure function already in existence, since now they need to know about min-content. Or adding a secondary function.