Skip formatting if data unchanged on Android #568
Conversation
|
Note: I feel like there could be some edge-cases where such optimization leads to incorrect result but I wasn't able to find one so far. This needs a bit of thought prior to merging. @289Adam289 Could you please help me test this PR for regressions? Thanks in advance. |
|
Sure, happy to help. I will get to it soon |
| if (text.equals(mPrevText) && markdownRanges == mPrevMarkdownRanges && markdownStyle == mPrevMarkdownStyle) { | ||
| // Use shallow comparison of markdown ranges and markdown style | ||
| // to optimistically skip removing and applying the same spans | ||
| return; | ||
| } |
There was a problem hiding this comment.
Isn't there anything that could alter the styles in between format passes? I'm not sure how it will behave with the commit hook when the text doesn't change but the node is cloned.
|
BUG:
Screen.Recording.2024-12-11.at.12.58.30.mov |
|
BUG: Screen.Recording.2024-12-11.at.12.59.46.movI've confirmed that both bug are not reproducible on main and are regressions from this PR |
|
Thanks @289Adam289 for testing out this PR and finding the regressions. As for #568 (comment), right now I don't have any idea on how to modify the logic to avoid this bug. Let's keep this PR as draft for now. |
Details
This PR skips removing and applying spans on Android if the text, ranges and styles are unchanged to previous run of
MarkdownFormatteras span operations are relatively time-consuming on Android.In particular, this lets us save some time during mounting phase as per systraces below:
Before
After
Related Issues
GH_LINK
Manual Tests
Linked PRs