Skip to content

fix: stabilize editor layout sizing to prevent large-file open slowdown#2769

Merged
gsantner merged 4 commits intogsantner:masterfrom
lrq3000:fix-pr/big-file-speed-regression
Mar 19, 2026
Merged

fix: stabilize editor layout sizing to prevent large-file open slowdown#2769
gsantner merged 4 commits intogsantner:masterfrom
lrq3000:fix-pr/big-file-speed-regression

Conversation

@lrq3000
Copy link
Copy Markdown
Contributor

@lrq3000 lrq3000 commented Mar 14, 2026

Summary

  • Fixed a major speed performance regression on startup of opening large files and opening the keyboard in such large files, which was introduced by layout changes in PR Quick filter all files, Share files into markor #2521 (isolated via bisect to commit 64ff85b6 -- commit description "Better layout") merged in between v2.14.1 and v2.15.0.
  • Root cause was layout-pass churn during editor initialization (unstable height sizing and repeated relayout pressure on large text/highlighting startup path).
  • Restored stable one-shot min-height initialization and adjusted editor layout params to avoid reintroducing expensive startup relayouts.

Details

  • DocumentEditAndViewFragment now uses one-shot min-height synchronization during startup instead of persistent per-layout updates.
  • Added concise guardrail comments in the hot path to explicitly prevent future reintroduction of layout-listener-based min-height updates.
  • document__fragment__edit.xml editor sizing was adjusted to align with the stable pre-regression layout behavior.

Related issues

Fixes #2739, #2716, #2652.

Test Build

The test build (Marder) is available here: https://github.com/lrq3000/markor/actions/runs/23170909092

Authorship

This was made with an AI agentic co-development using git-bisect and OpenCode with ChatGPT Codex-5.3

Additional comments

Please let me know if the fix is alright for you guys, I don't know the downstream consequences of these changes since I don't know the codebase.

I would suggest to add an automated performance test in the ci to ensure there is no more speed regression in the future. I can try to implement one as I was the one implementing these speed regression tests in the tqdm python module, so I have some experience on how to do that reliably (using AST decomposition instead of a timer, because otherwise the tests will randomly fail depending on the CI cloud server usage and that's very bothersome). But I am not sure this is possible in Android, but I can try. Just let me know if you would be interested in adding this to the project if my endeavor is successful.

Note that in any case, I would work on the automated performance unit test as a separate PR, because I know from experience that this is a very complex kind of unit test, very time consuming to make robustly, and so it's better in a separate PR (and I may not have the time to ever tackle it so the current fix should not depend on the performance unit test).

Todo

lrq3000 added 2 commits March 14, 2026 07:04
Restore one-shot min-height initialization and avoid per-layout min-height churn, with guardrail comments to prevent reintroducing startup relayout regressions.

This fixes a major speed regression introduced in commit 64ff85b in PR gsantner#2521 (squashed into b2f4407) that made loading of big files and opening the keyboard incredibly slow and proportional to the file size.

Signed-off-by: Stephen L. <LRQ3000@gmail.com>
Using OpenCode with ChatGPT Codex-5.3 + git-bisect
Signed-off-by: Stephen L. <LRQ3000@gmail.com>
…ditorMinHeightOnce() to do one-shot editor min-height sync to prevent large-file open slowdown + cover more cases as originally intended in 64ff85b

Signed-off-by: Stephen L. <LRQ3000@gmail.com>
@lrq3000
Copy link
Copy Markdown
Contributor Author

lrq3000 commented Mar 16, 2026

I pushed a small update to get even closer to feature parity with the original intent in the 64ff85b commit that introduced the speed regression. Unfortunately, this commit's description is quite indescript ("Better layout"), so I used AI to try to infer the original intents and what features may be lost with the speed regression fix, here is the chatlog:

What `64ff85b6` likely intended
- Improve **editor layout consistency** in wrapped/unwrapped modes (especially when moving editor in/out of `HorizontalScrollView` in `setWrapState`).
- Make the editor occupy space more predictably using weighted params (`0dp + weight`) in `document__fragment__edit.xml`.
- Simplify bottom margin handling for the active edit container (`showHideActionBar` path).
- Remove visual jank from startup layout behavior (though the implementation chosen caused performance issues).

Does your current fix contradict that intent?
- **Mostly no.** Your fix removes the harmful part (persistent/per-layout min-height churn) but keeps the core UX behavior:
  - Wrap/unwrapped mode switching is still there in `app/src/main/java/net/gsantner/markor/activity/DocumentEditAndViewFragment.java`.
  - Action-bar bottom spacing behavior remains.
  - Viewport fill for short docs is still handled via one-shot min-height sync.

----

what are the features we lose with this fix compared to the original intent in the commit 64ff85b6?

----

Very little is lost, and mostly in edge-cases.

Compared to original `64ff85b6` intent, what you potentially lose now:

- No continuous live min-height re-sync on every later container resize event.
  - Possible minor impact on rare cases like split-screen resize / keyboard timing / rotation after initial open.
- Initial wrapped layout no longer uses the earlier weighted `0dp` width experiment.
  - We kept the known-fast `match_parent` start config.

What you **keep** from the original intent:

- Wrap/unwrapped mode switching behavior.
- Editor reparenting into/out of horizontal scroll for no-wrap mode.
- Selection restore + smooth transition behavior.
- Action-bar bottom margin handling.
- Overall “better layout” UX, without the performance regression.

So the tradeoff is essentially:  
**we gave up continuous auto-resync behavior (which was costly) to keep startup/open performance stable.**

----

What does this imply in practice? What are scenarios, use cases, where we will see a difference, and what difference?

----

Great question. In practice, here’s where you may notice differences now:

- **Normal use (open/edit large files, toggle wrap, type, save):** no negative difference; this is where you gained the big speed win.
- **Keyboard show/hide after initial open:** mostly same; in rare devices you might see a bit more/less blank space below text until next explicit UI update.
- **Rotation / split-screen resize while the editor is already open:** this is the main edge case; height may not re-sync as “continuously” as before, so viewport fill can be slightly stale momentarily.
- **Very short documents:** occasionally you may see the editor area not perfectly “stretched” after dynamic container changes until another trigger (e.g. wrap toggle, action bar visibility change) runs.
- **Wrap toggle behavior itself:** still good, because we kept explicit one-shot resync on wrap transitions.

So the practical tradeoff is:
- **Before:** continuous auto-resync everywhere (but expensive, causing severe slowdown on big files).
- **Now:** explicit one-shot resync at key moments (fast and stable), with only rare post-resize visual edge cases.

I tested several of those edge cases, especially rotation after a large note is already opened in the editor, and I saw no noticeably slowdown compared to v2.14.1 nor UI discrepancies compared to v2.16.0, so in practice the "main edge case" is handled in my field tests.

Copy link
Copy Markdown
Owner

@gsantner gsantner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @lrq3000 ,
thank you. I tried it, and in contrast to #2770 very risky changes didn't notice something off in this one.
I will merge this, thank you!

@gsantner gsantner merged commit 45363ff into gsantner:master Mar 19, 2026
gsantner added a commit that referenced this pull request Mar 19, 2026
…wn (PR #2769, closes #2739 #2652 #2716)

* fix: stabilize editor layout sizing to prevent large-file open slowdown

Restore one-shot min-height initialization and avoid per-layout min-height churn, with guardrail comments to prevent reintroducing startup relayout regressions.

This fixes a major speed regression introduced in commit 64ff85b in PR #2521 (squashed into b2f4407) that made loading of big files and opening the keyboard incredibly slow and proportional to the file size.

* docs: add lrq3000 in CONTRIBUTORS.md
* fix: Refactor speed regression fix duplications into a function syncEditorMinHeightOnce() to do one-shot editor min-height sync to prevent large-file open slowdown + cover more cases as originally intended in 64ff85b

Signed-off-by: Stephen L. <LRQ3000@gmail.com>
gsantner added a commit that referenced this pull request Mar 19, 2026
…wn (PR #2769, closes #2739 #2652 #2716)

* fix: stabilize editor layout sizing to prevent large-file open slowdown

Restore one-shot min-height initialization and avoid per-layout min-height churn, with guardrail comments to prevent reintroducing startup relayout regressions.

This fixes a major speed regression introduced in commit 64ff85b in PR #2521 (squashed into b2f4407) that made loading of big files and opening the keyboard incredibly slow and proportional to the file size.

* docs: add lrq3000 in CONTRIBUTORS.md
* fix: Refactor speed regression fix duplications into a function syncEditorMinHeightOnce() to do one-shot editor min-height sync to prevent large-file open slowdown + cover more cases as originally intended in 64ff85b

Signed-off-by: Stephen L. <LRQ3000@gmail.com>
@lrq3000
Copy link
Copy Markdown
Contributor Author

lrq3000 commented Mar 19, 2026

@gsantner Awesome, thank you so much for fast-tracking the merge of this PR!

Yes #2770 is riskier so I will do extensive testing myself before tagging it as ready, and even then I understand there will have to be a decision whether or not to merge it in. So far the progress is good and I think we may get a stable release with features parity for the large text editor soon-ish :-)

Also if you are open to getting donations, please let me know how you would like to proceed, you can also contact me at my e-mail LRQ3000 at gmail dot com :-) (PS: don't worry, my e-mail is already displayed on every commits messages so I don't care much at this point).

_verticalScrollView.post(ensureMinHeight);
// Keep this as a one-shot min-height sync. Do not use a persistent layout listener here,
// otherwise large documents trigger repeated relayout work during startup.
syncEditorMinHeightOnce(_verticalScrollView);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the window is resized (i.e. in multi window mode)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not test but it should work just like Markor v2.14.1 and earlier so it should be fine. I will give it a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants