Repository navigation
Commit e7766ab
Keep paragraph content and measured layouts across unchanged clones (#59063)
Summary:
When Yoga lays out a container again, it clones every child whose layout it touches, including unchanged siblings. A `ParagraphShadowNode` cloned that way (no new props, no new children) drops its memoized `content_` (the attributed string) and `measuredLayouts_`.
**Current behavior.** In `layout()` the clone rebuilds its `AttributedString` and compares it with the one in its `ParagraphState`. After the paragraph's first commit the comparison fails even though nothing changed. `Fragment::operator==` compares `parentShadowView.layoutMetrics`, and the string stored in state captured those metrics before the paragraph had been laid out. So `updateStateIfNeeded` creates a new state for every sibling paragraph:
- On Android each sibling then gets an `UPDATE_STATE` mount instruction. The UI thread rebuilds its Spannable, calls `setText`, lays the text out again and redraws it.
- A trace of "mount 1000 labels, then change one" shows 1000 `ReactTextViewManager.updateState`, 1000 `setText` and 1000 `onDraw` calls for the one change.
- With `enablePreparedTextLayout`, the clone also has no measured layout to reuse, so every sibling is laid out again on the JS thread.
**With this change.** A clone whose fragment has neither props nor children keeps `content_` and `measuredLayouts_`. `getContent` drops both if the font-size multiplier or layout direction they were built for no longer matches. Unchanged siblings then produce no state update and no re-layout.
A kept layout is only reused at the pixel density it was prepared for. After a density change the paragraph prepares a new layout, so the state refresh from #59049 (`fixTextLayoutOnPixelDensityChange`) still applies with `enablePreparedTextLayout`.
Content with attachments (inline views inside the text) is not kept: the attachments point at the children the content was built from, and `layout()` replaces those children with laid-out clones, so in a later revision they would point at nodes only the previous revision owns.
**Why.** Any update inside a container with many text siblings (non-virtualized lists, settings screens, forms, tables) currently costs UI-thread work for every sibling, and with prepared text layout also a JS-thread layout for every sibling.
## Changelog:
[GENERAL] [FIXED] - Avoid state updates and re-layout of unchanged `Text` siblings when a sibling changes
Pull Request resolved: #59063
Test Plan:
- `./gradlew :packages:rn-tester:android:app:assembleDebug -PreactNativeArchitectures=arm64-v8a` and `yarn format-check-cpp` pass on this branch.
- Fantom could not be built locally: the bundled `nlohmann/json.hpp` fails with this machine's Apple clang. I'm relying on CI for it.
- Screenshots were pixel-identical before and after a sequence of updates (all text, one item, colour, font size, container width), in four configurations with and without `enablePreparedTextLayout`.
- iOS shares this code path but was not built or measured.
- The pixel-density check (last commit) was compiled against the Android build's flags with `-fsyntax-only` but not run; no density change was tested on a device.
- Galaxy A16 release build, cold start and 15 flings through two long feeds whose text contains inline views, 16 runs per variant: the first version of this PR crashed in 4 (SIGSEGV in `ParagraphShadowNode::layout`, reading an attachment the previous revision had released), the current version in none.
**Benchmarks.** RNTester release build with a text benchmark screen that is not part of this PR. Medians, main vs. this PR, variants interleaved.
Emulator (Pixel 10 Pro AVD, API 37), current `main` vs. this branch, 6 runs:
| scenario | metric | main | this PR | change |
|---|---|--:|--:|--:|
| 1000 labels, change one | UI-thread CPU | 21.8 ms | 5.2 ms | −76% |
| | time to pixels | 58.6 ms | 33.6 ms | −43% |
| 300 strings, change one | UI-thread CPU | 29.0 ms | 6.0 ms | −79% |
| | time to pixels | 49.3 ms | 32.5 ms | −34% |
| 1000 labels, change one, `enablePreparedTextLayout` | JS-thread CPU | 30.4 ms | 18.5 ms | −39% |
| | time to pixels | 66.2 ms | 49.1 ms | −26% |
| mounts | | | | no significant change |
Physical Samsung Galaxy A16, the same change on an earlier `main` (`d4d0813969c`), 8 runs. #59016 was applied in both arms.
| scenario | main | this change | change |
|---|--:|--:|--:|
| 1000 labels, change one right after mount: time to pixels | 222 ms | 78 ms | −65% |
| 300 strings, change one right after mount: time to pixels | 164 ms | 36 ms | −78% |
| 1000 labels, change one, prepared text: time to pixels | 228 ms | 73 ms | −68% |
| same: JS-thread CPU | 132 ms | 44 ms | −67% |
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Reviewed By: cipolleschi
Differential Revision: D126131800
Pulled By: javache
fbshipit-source-id: 88d934a7ecb188430c19a6c62135f4602da7b99f1 parent 631fd89 commit e7766ab
2 files changed
Lines changed: 50 additions & 12 deletions
File tree
- packages/react-native/ReactCommon/react/renderer/components/text
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
62 | 79 | | |
63 | 80 | | |
64 | 81 | | |
| |||
69 | 86 | | |
70 | 87 | | |
71 | 88 | | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
72 | 92 | | |
73 | | - | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
74 | 109 | | |
75 | 110 | | |
76 | 111 | | |
77 | 112 | | |
78 | 113 | | |
79 | 114 | | |
80 | 115 | | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
| 116 | + | |
85 | 117 | | |
86 | 118 | | |
87 | 119 | | |
| |||
204 | 236 | | |
205 | 237 | | |
206 | 238 | | |
207 | | - | |
| 239 | + | |
| 240 | + | |
208 | 241 | | |
209 | 242 | | |
210 | 243 | | |
| |||
216 | 249 | | |
217 | 250 | | |
218 | 251 | | |
219 | | - | |
| 252 | + | |
| 253 | + | |
220 | 254 | | |
221 | 255 | | |
222 | 256 | | |
| |||
245 | 279 | | |
246 | 280 | | |
247 | 281 | | |
| 282 | + | |
| 283 | + | |
248 | 284 | | |
249 | | - | |
| 285 | + | |
| 286 | + | |
250 | 287 | | |
251 | 288 | | |
252 | 289 | | |
| |||
336 | 373 | | |
337 | 374 | | |
338 | 375 | | |
339 | | - | |
| 376 | + | |
340 | 377 | | |
341 | 378 | | |
342 | 379 | | |
| |||
353 | 390 | | |
354 | 391 | | |
355 | 392 | | |
356 | | - | |
| 393 | + | |
357 | 394 | | |
358 | 395 | | |
359 | 396 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
117 | 117 | | |
118 | 118 | | |
119 | 119 | | |
120 | | - | |
| 120 | + | |
| 121 | + | |
121 | 122 | | |
122 | | - | |
| 123 | + | |
123 | 124 | | |
124 | 125 | | |
125 | 126 | | |
| |||
0 commit comments