From 4ee72cf8485ddaa448a711eb94f6f95db8582cb5 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Sun, 16 Aug 2026 12:18:01 -0400 Subject: [PATCH] Doc: mark Phase 3 complete (10 MB validated, shaper leak fixed, 50 MB limit) --- doc/development_plan.md | 83 ++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/doc/development_plan.md b/doc/development_plan.md index d894c5f..00af330 100644 --- a/doc/development_plan.md +++ b/doc/development_plan.md @@ -1,9 +1,12 @@ # Development Plan: reach a lean, usable Android text editor -Status: v2, 2026-08-16 (Phase 2 on-device IME validation complete). Written -against the **live** repo `/home/gmp/pad`. v1 (the widget-rebuild plan) is -superseded — see §12 for why. Phases 0–2 done; Phase 3 (large-file + the -viewport-on-open bug) is next. +Status: v3, 2026-08-16 (Phases 0–3 complete). Written against the **live** repo +`/home/gmp/pad`. v1 (the widget-rebuild plan) is superseded — see §12 for why. +Phases 0–3 done: single-owner no-lock architecture, Android IME wiring, on-device +IME validation (passing), viewport-on-open fix, chunked-buffer drift fix, the +whole-file shaper memory-leak fix, and a measured 50 MB size limit. Remaining: +rapid-synthetic-IME-commit desync hardening and real-device swipe/autocorrect +sign-off. ## 1. Decision summary (updated) @@ -180,11 +183,32 @@ Observation loop that worked (no state-dump flag needed in the end): logcat (`imeDebugLog` + `HandleKeyDown` + `VisibleByteRange`), autosaved-file diff via `adb shell cat`, and screenshots (vision, auxiliary). -### Phase 3 — large-file validation + honest size limit -1. Open a real **10 MB** file via the chunked buffer; verify smooth scroll + edit - and measure RAM on-device. -2. Set the documented limit from that measurement; guard larger files with a - clear "too large to edit" state (browser can still list/preview). +### Phase 3 — large-file validation + honest size limit — DONE +1. **Open a real 10 MB file** via the chunked buffer: opens in ~121 ms + (stat→read→index), renders correctly, smooth scroll. ✓ +2. **Fixed the whole-file shaper leak** (the real 1 GB memory bug): with word + wrap on (the default), `VisibleByteRange` used the previously-shaped + `GlyphLayout.VisualLineStarts` to bound the visible range. That layout only + covers the visible window (~50 lines), not the document, so once the + viewport's line count exceeded the window's visual-line count the range fell + back to `end=fileLen` and the shaper laid out the **entire file** every frame. + Gio's shaper `document.reset()` keeps the backing-array cap, so memory grew + to the largest layout ever shaped and never shrank (~1.1 GB PSS for 10 MB, + OOM-killed under scroll). Fix: always derive the range from the real-line + `LineIndex`; word wrap needs no separate path (each real line yields ≥1 + visual line). Now ~150 MB PSS / ~230 MB RSS at steady state, flat under + scroll. (commit `c79c142`) +3. **Fixed the pre-existing LineIndex storage mismatch**: `EditorLayout` read + `TheState.Editor.LineIndex` (never set; `SetLineIndex` had zero callers) for + `maxScroll`/`scrollByteOffset`, always using the huge pre-index estimate. Now + reads `cb.LineIndex`; the dead field + setter are removed. +4. **Honest size limit = 50 MB** (`MaxEditableFileSize`), measured on-device + (10 MB → ~150 MB PSS; 50 MB extrapolates to a few hundred MB, fine on a + phone). Larger files get a "too large to edit" state; the browser still + lists them. ✓ +5. **Remaining (not blocking usability):** rapid *synthetic* IME commit desync + (only re-push the snippet when it changes) and real-device swipe/autocorrect + sign-off. ### Phase 4 — the v1 simplifications, now that it's usable (optional, later) Only after the app is usable: replace globals (`TheState`, `ui.OpenFile`) with @@ -203,8 +227,9 @@ already chose the better option — a **chunked buffer** (only viewport ± windo chunks resident, async prefetch, dirty-chunk protection, async line index). So the decision is now: **validate the existing chunked buffer against a real 10 MB file (Phase 3) and set the limit from the measurement.** No fork of Gio is on the -critical path. If the chunked buffer proves out (likely), 10 MB+ editing is in -reach without the 0.5 GB/MB wall that kills `widget.Editor`. +critical path. The chunked buffer proved out on a real 10 MB file (Phase 3, §5): +10 MB+ editing works with ~linear, bounded memory, without the 0.5 GB/MB wall +that kills `widget.Editor`. ## 7. Spec deltas (to write into spec.md) @@ -283,28 +308,24 @@ within ~1 s. Repo: `go test -race ./...` green. reports caret px, so we can assert it against expected values in tests. - **Channel model + `-race`** (Phase 0) — **resolved** in `58725a5` (§13); `-race` is green and is now a standing regression gate. -- **Chunked buffer at 10 MB** (Phase 3): unproven on-device; the measurement is - the gate for the size claim. -- **Chunked buffer fixed-slot drift** (Phase 3, discovered during IME work): the - buffer models chunk *i* as bytes `[i*chunkSize, (i+1)*chunkSize)`. That holds - for a freshly loaded file and for read-only display, but a **length-changing - edit** (insert/delete) shifts the bytes after the edit point, so the - fixed-slot offsets no longer line up for any *not-yet-loaded* tail chunk. In - practice: (a) files fully in memory are fine (all chunks loaded, `FullContent` - concatenates by index); (b) the caret region is always loaded, so **IME commits - at the caret are correct**; (c) a *large* file with an *unloaded* tail that is - then edited and scrolled into is where it breaks (lazy `loadChunk` reads stale - disk, since the file is only rewritten whole on save). A proper fix is a rope - or an offset-index structure. Phase 3 must either avoid this case (cap edits - to loaded region / flush before loading a shifted tail) or replace the slot - model. Until then the honest size limit should assume the file is small enough - to stay fully in memory (the ~50 MB target is borderline; see §6). +- **Chunked buffer at 10 MB** (Phase 3) — **resolved**: opens a real 10 MB file + in ~121 ms, renders and scrolls smoothly, ~150 MB PSS at steady state. The 50 MB + limit is set from this measurement (§5 Phase 3, commit `c79c142`). +- **Chunked buffer fixed-slot drift** (Phase 3) — **resolved**: the buffer was + rewritten off the fixed `[i*chunkSize, (i+1)*chunkSize)` slot model. It now + holds an ordered `[][]byte` chunk slice with **prefix-sum byte offsets** and + full-loads in-range files on open (no lazy loading, no eviction), so byte↔chunk + mapping stays correct after length-changing edits. A rope is no longer needed + for the 50 MB target. +- **Whole-file shaper memory leak** (Phase 3) — **resolved**: `VisibleByteRange` + no longer falls back to `end=fileLen` via the window-only `VisualLineStarts`; + the range is always bounded by the real-line `LineIndex`. Memory is now stable + and scales ~linearly with file size (commit `c79c142`). - **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect sign-off needs a real device with a real IME. -- **Viewport opens at EOF** (Phase 2, open): the opening tap leaks into the editor - and scrolls a short file past its content → blank editor until you scroll to top. - Blocks a clean first impression; fix in Phase 3 (ignore the opening tap / clamp - scroll for files that fit the viewport). See §5 Phase 2 bugs. +- **Viewport opens at EOF** (Phase 2) — **resolved**: the opening tap/scroll is + swallowed for ~300 ms via a `justOpenedAt` window and `ScrollOffset` is + re-clamped in `EditorLayout` as a safety net (commit `3460ef3`). ## 12. Why v1 (widget rebuild) is now a fallback, not the plan v1 was written against a stale snapshot (May 31) and concluded "delete