Doc: mark Phase 3 complete (10 MB validated, shaper leak fixed, 50 MB limit)
This commit is contained in:
parent
c79c142397
commit
4ee72cf848
|
|
@ -1,9 +1,12 @@
|
||||||
# Development Plan: reach a lean, usable Android text editor
|
# Development Plan: reach a lean, usable Android text editor
|
||||||
|
|
||||||
Status: v2, 2026-08-16 (Phase 2 on-device IME validation complete). Written
|
Status: v3, 2026-08-16 (Phases 0–3 complete). Written against the **live** repo
|
||||||
against the **live** repo `/home/gmp/pad`. v1 (the widget-rebuild plan) is
|
`/home/gmp/pad`. v1 (the widget-rebuild plan) is superseded — see §12 for why.
|
||||||
superseded — see §12 for why. Phases 0–2 done; Phase 3 (large-file + the
|
Phases 0–3 done: single-owner no-lock architecture, Android IME wiring, on-device
|
||||||
viewport-on-open bug) is next.
|
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)
|
## 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
|
(`imeDebugLog` + `HandleKeyDown` + `VisibleByteRange`), autosaved-file diff via
|
||||||
`adb shell cat`, and screenshots (vision, auxiliary).
|
`adb shell cat`, and screenshots (vision, auxiliary).
|
||||||
|
|
||||||
### Phase 3 — large-file validation + honest size limit
|
### Phase 3 — large-file validation + honest size limit — DONE
|
||||||
1. Open a real **10 MB** file via the chunked buffer; verify smooth scroll + edit
|
1. **Open a real 10 MB file** via the chunked buffer: opens in ~121 ms
|
||||||
and measure RAM on-device.
|
(stat→read→index), renders correctly, smooth scroll. ✓
|
||||||
2. Set the documented limit from that measurement; guard larger files with a
|
2. **Fixed the whole-file shaper leak** (the real 1 GB memory bug): with word
|
||||||
clear "too large to edit" state (browser can still list/preview).
|
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)
|
### Phase 4 — the v1 simplifications, now that it's usable (optional, later)
|
||||||
Only after the app is usable: replace globals (`TheState`, `ui.OpenFile`) with
|
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
|
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
|
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
|
(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
|
critical path. The chunked buffer proved out on a real 10 MB file (Phase 3, §5):
|
||||||
reach without the 0.5 GB/MB wall that kills `widget.Editor`.
|
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)
|
## 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.
|
reports caret px, so we can assert it against expected values in tests.
|
||||||
- **Channel model + `-race`** (Phase 0) — **resolved** in `58725a5` (§13);
|
- **Channel model + `-race`** (Phase 0) — **resolved** in `58725a5` (§13);
|
||||||
`-race` is green and is now a standing regression gate.
|
`-race` is green and is now a standing regression gate.
|
||||||
- **Chunked buffer at 10 MB** (Phase 3): unproven on-device; the measurement is
|
- **Chunked buffer at 10 MB** (Phase 3) — **resolved**: opens a real 10 MB file
|
||||||
the gate for the size claim.
|
in ~121 ms, renders and scrolls smoothly, ~150 MB PSS at steady state. The 50 MB
|
||||||
- **Chunked buffer fixed-slot drift** (Phase 3, discovered during IME work): the
|
limit is set from this measurement (§5 Phase 3, commit `c79c142`).
|
||||||
buffer models chunk *i* as bytes `[i*chunkSize, (i+1)*chunkSize)`. That holds
|
- **Chunked buffer fixed-slot drift** (Phase 3) — **resolved**: the buffer was
|
||||||
for a freshly loaded file and for read-only display, but a **length-changing
|
rewritten off the fixed `[i*chunkSize, (i+1)*chunkSize)` slot model. It now
|
||||||
edit** (insert/delete) shifts the bytes after the edit point, so the
|
holds an ordered `[][]byte` chunk slice with **prefix-sum byte offsets** and
|
||||||
fixed-slot offsets no longer line up for any *not-yet-loaded* tail chunk. In
|
full-loads in-range files on open (no lazy loading, no eviction), so byte↔chunk
|
||||||
practice: (a) files fully in memory are fine (all chunks loaded, `FullContent`
|
mapping stays correct after length-changing edits. A rope is no longer needed
|
||||||
concatenates by index); (b) the caret region is always loaded, so **IME commits
|
for the 50 MB target.
|
||||||
at the caret are correct**; (c) a *large* file with an *unloaded* tail that is
|
- **Whole-file shaper memory leak** (Phase 3) — **resolved**: `VisibleByteRange`
|
||||||
then edited and scrolled into is where it breaks (lazy `loadChunk` reads stale
|
no longer falls back to `end=fileLen` via the window-only `VisualLineStarts`;
|
||||||
disk, since the file is only rewritten whole on save). A proper fix is a rope
|
the range is always bounded by the real-line `LineIndex`. Memory is now stable
|
||||||
or an offset-index structure. Phase 3 must either avoid this case (cap edits
|
and scales ~linearly with file size (commit `c79c142`).
|
||||||
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).
|
|
||||||
- **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect
|
- **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect
|
||||||
sign-off needs a real device with a real IME.
|
sign-off needs a real device with a real IME.
|
||||||
- **Viewport opens at EOF** (Phase 2, open): the opening tap leaks into the editor
|
- **Viewport opens at EOF** (Phase 2) — **resolved**: the opening tap/scroll is
|
||||||
and scrolls a short file past its content → blank editor until you scroll to top.
|
swallowed for ~300 ms via a `justOpenedAt` window and `ScrollOffset` is
|
||||||
Blocks a clean first impression; fix in Phase 3 (ignore the opening tap / clamp
|
re-clamped in `EditorLayout` as a safety net (commit `3460ef3`).
|
||||||
scroll for files that fit the viewport). See §5 Phase 2 bugs.
|
|
||||||
|
|
||||||
## 12. Why v1 (widget rebuild) is now a fallback, not the plan
|
## 12. Why v1 (widget rebuild) is now a fallback, not the plan
|
||||||
v1 was written against a stale snapshot (May 31) and concluded "delete
|
v1 was written against a stale snapshot (May 31) and concluded "delete
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user