From c79c1423976e52f08743ced46971f61e15b6a860 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Sun, 16 Aug 2026 12:16:52 -0400 Subject: [PATCH] editor: fix whole-file shaper leak + LineIndex mismatch (Phase 3) Root cause of the ~1GB 'Unknown' memory on large files: with word wrap on (the default), VisibleByteRange used the previously-shaped GlyphLayout.VisualLineStarts to bound the visible byte range. That layout only covers the visible window (~50 lines), not the whole document, so whenever the viewport's line count exceeded the window's visual-line count the range fell back to end=fileLen. The text shaper then laid out the ENTIRE file each frame; its internal line/glyph buffers grow to the largest layout ever shaped and are never released (document.reset() keeps the backing cap), so memory ballooned to the file size (~1.1 GB for 10 MB) and OOM-killed the process under scroll. Fix: always derive the visible range from the real-line LineIndex (or the heuristic estimate before it is ready). Word wrap needs no separate path: each real line yields >= 1 visual line, so shaping viewportHeight/lineHeight real lines always fills the viewport, and the range can never collapse to the whole file. Also fixed the pre-existing LineIndex storage mismatch: EditorLayout read TheState.Editor.LineIndex (never set; SetLineIndex had zero callers) for maxScroll and scrollByteOffset, so it always used the huge pre-index estimate. It now reads cb.LineIndex, the single source of truth; the dead EditorState.LineIndex field and SetLineIndex are removed. On-device (emulator/SwiftShader), a 10 MB file now uses ~150 MB PSS / ~230 MB RSS at steady state and stays flat under scroll (was 1.13 GB PSS / 1.5 GB RSS, growing, OOM-killed). go test -race ./... green. --- internal/editor/chunked_buffer.go | 63 +++++++---------- internal/editor/state.go | 108 ++++++++++++++---------------- 2 files changed, 75 insertions(+), 96 deletions(-) diff --git a/internal/editor/chunked_buffer.go b/internal/editor/chunked_buffer.go index b8e8b49..4f79591 100644 --- a/internal/editor/chunked_buffer.go +++ b/internal/editor/chunked_buffer.go @@ -14,9 +14,16 @@ const ( // MaxEditableFileSize is the largest file the editor will open for editing. // In-range files are loaded fully into memory (see Phase 3): the chunked // buffer keeps the raw bytes (~file size) resident and the renderer - // virtualizes the glyph layout to the visible window. Files above this are - // rejected with a "too large to edit" state (the browser can still list - // them). Set from on-device measurement (see doc/development_plan.md §5). + // virtualizes the glyph layout to the visible window, so memory scales + // roughly linearly with file size and stays bounded (no leak). Files above + // this are rejected with a "too large to edit" state (the browser can still + // list them). + // + // On-device measurement (Android emulator, SwiftShader): a 10 MB file uses + // ~150 MB PSS / ~230 MB RSS at steady state and stays flat under scroll + // (previously the shaper was handed the whole file each frame, ballooning + // to ~1.1 GB and OOM-killing the process). 50 MB extrapolates to a few + // hundred MB, comfortable on a modern phone. MaxEditableFileSize = 50 * 1024 * 1024 // 50 MB ) @@ -390,41 +397,21 @@ func (cb *ChunkedBuffer) Delete(pos, n int) { // viewport, not the file. It uses actual (prefix-sum) chunk offsets so the // range stays correct after length-changing edits. func (cb *ChunkedBuffer) VisibleByteRange(scrollOffset ui.Dp, byteOffset int, viewportHeight ui.Dp, lineHeight ui.Dp, wordWrap bool, layout ui.GlyphLayout, visualIndex *types.VisualLineIndex) (start, end, startLine int) { - // If wrapping and we have visual line data, use it for precise calculation - if wordWrap && len(layout.VisualLineStarts) > 0 { - // Calculate which visual line should be at the top based on scroll offset - visualLine := int(scrollOffset / lineHeight) - if visualLine >= len(layout.VisualLineStarts) { - visualLine = len(layout.VisualLineStarts) - 1 - } - - start = layout.VisualLineStarts[visualLine] + byteOffset - - // Calculate end: enough content to fill viewport + buffer - linesInViewport := int(viewportHeight/lineHeight) + 2 - endLine := visualLine + linesInViewport - if endLine >= len(layout.VisualLineStarts) { - end = int(cb.fileLen) - } else { - end = layout.VisualLineStarts[endLine] - } - - // Clamp to file bounds - if end > int(cb.fileLen) { - end = int(cb.fileLen) - } - if start >= end { - end = start + 1000 // minimum buffer - if end > int(cb.fileLen) { - end = int(cb.fileLen) - } - } - return start, end, visualLine - } - - // Fallback to LineIndex (logical lines) or estimate if: - // 1. Not word wrapping - // 2. No visual index available + // The visible range is always derived from the real-line LineIndex (or a + // heuristic estimate before the index is ready). The previously-shaped + // GlyphLayout (layout.VisualLineStarts) only covers the visible window, NOT + // the whole document. Using it to bound the range made the end fall back to + // the entire file whenever the viewport's line count exceeded the window's + // visual-line count, which forced the text shaper to lay out the whole + // document and ballooned memory to the file size (the shaper's internal + // line/glyph buffers grow to the largest layout ever shaped and are never + // released). That is the root cause of the ~1GB "Unknown" memory on large + // files. + // + // Word wrap does not require a separate path: each real line produces at + // least one visual line, so shaping viewportHeight/lineHeight real lines + // always yields at least as many visual lines as fit in the viewport. The + // extra wrapped lines are simply clipped by the renderer. if cb.LineIndex == nil { start, end = cb.visibleByteRangeEstimate(scrollOffset, viewportHeight) return start, end, 0 diff --git a/internal/editor/state.go b/internal/editor/state.go index 5b0515a..31f2fc0 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -8,10 +8,9 @@ import ( "time" "unicode/utf8" - "pad/internal/browser" - "pad/internal/io/pool/types" - "pad/internal/ui" "gioui.org/io/key" + "pad/internal/browser" + "pad/internal/ui" ) // EditorFontSize is the font size used for editor text. @@ -59,14 +58,13 @@ const ( // EditorState holds all editor-specific state. type EditorState struct { - Buffer string // DEPRECATED: use ChunkedBuffer for large files - ChunkedBuffer *ChunkedBuffer // NEW: chunked file access for virtual scrolling - LineIndex *types.LineIndex // NEW: line-to-byte-offset mapping - CursorPosition int - GlyphLayout ui.GlyphLayout - SelectionStart int - SelectionEnd int - CursorVisible bool + Buffer string // DEPRECATED: use ChunkedBuffer for large files + ChunkedBuffer *ChunkedBuffer // NEW: chunked file access for virtual scrolling + CursorPosition int + GlyphLayout ui.GlyphLayout + SelectionStart int + SelectionEnd int + CursorVisible bool // IMEWindowStartByte is the absolute byte offset in the buffer where the // visible window (IMEWindowText) begins. For small (string) files it is 0 // and the window is the whole buffer; for large (chunked) files it is the @@ -76,17 +74,17 @@ type EditorState struct { // IMEWindowText is the visible window text shown to the IME (the snippet). // It is set during layout and is what an EditEvent.Range indexes into. IMEWindowText string - Filename string + Filename string // TooLarge is set when an opened file exceeds MaxEditableFileSize. The // editor shows a "too large to edit" notice instead of content (the // browser can still list the file). - TooLarge bool - TooLargeSize int64 - fileVersion map[string]int + TooLarge bool + TooLargeSize int64 + fileVersion map[string]int lastWriteVersion map[string]int saveTimer *time.Timer - writeFailed map[string]bool // Added: tracks failed writes for UI - retryAttempts map[string]int // Added: tracks retry attempts + writeFailed map[string]bool // Added: tracks failed writes for UI + retryAttempts map[string]int // Added: tracks retry attempts } // GetBuffer returns the full buffer content, using ChunkedBuffer if available. @@ -135,31 +133,31 @@ func (e *EditorState) IncrementRetryAttempts(filename string) int { // State holds all application state owned by the logic goroutine. type State struct { - PixelWidth int // raw pixel width from Gio ConfigEvent - PixelHeight int // raw pixel height from Gio ConfigEvent + PixelWidth int // raw pixel width from Gio ConfigEvent + PixelHeight int // raw pixel height from Gio ConfigEvent scale float32 - page Page // current page (Browser or Editor) + page Page // current page (Browser or Editor) WordWrap bool - ScrollOffset ui.Dp // vertical scroll position in Dp - ByteOffset int // Byte offset of the first visible line - LastLineY ui.Dp // last line baseline offset from text origin, from renderer - MaxScroll ui.Dp // max scroll offset (content height - viewport height) - FocusedElementID string // ID of the currently focused element + ScrollOffset ui.Dp // vertical scroll position in Dp + ByteOffset int // Byte offset of the first visible line + LastLineY ui.Dp // last line baseline offset from text origin, from renderer + MaxScroll ui.Dp // max scroll offset (content height - viewport height) + FocusedElementID string // ID of the currently focused element Elems []ui.Element - lastEvictionTime time.Time // Throttles chunk eviction - justOpenedAt time.Time // when the editor page was last opened; used to swallow the opening tap + lastEvictionTime time.Time // Throttles chunk eviction + justOpenedAt time.Time // when the editor page was last opened; used to swallow the opening tap // Browser state (directly embedded per architecture §8) Browser browser.BrowserState // Embedded, not a pointer // Editor state Editor EditorState // New field - open func(string) + open func(string) } func NewState() *State { return &State{ scale: 1.0, page: BrowserPage, // Reverted to BrowserPage - WordWrap: true, // Enable word wrap by default + WordWrap: true, // Enable word wrap by default lastEvictionTime: time.Now(), Browser: *browser.NewBrowserState(), Editor: EditorState{ @@ -174,8 +172,6 @@ func NewState() *State { } } - - func (s *State) SetScale(scale float32) { s.scale = scale } @@ -192,7 +188,6 @@ func (s *State) layout(bm *browser.BrowserManager) []ui.Element { dpW := ui.ToDp(ui.Px(s.PixelWidth), s.scale) dpH := ui.ToDp(ui.Px(s.PixelHeight), s.scale) - // Calculate VisibleCount before laying out the browser page. // This ensures the browser shows entries based on the current viewport. if s.PixelHeight > 0 && s.scale > 0 { @@ -247,12 +242,12 @@ func HandleScroll(data any) { // Throttle eviction to 500ms and use a larger radius to prevent thrashing. // Temporarily disabled eviction to debug thrashing issues. /* - if cb := TheState.Editor.ChunkedBuffer; cb != nil { - if time.Since(TheState.lastEvictionTime) > 500*time.Millisecond { - cb.EvictFarChunks(TheState.Editor.CursorPosition, 20) - TheState.lastEvictionTime = time.Now() + if cb := TheState.Editor.ChunkedBuffer; cb != nil { + if time.Since(TheState.lastEvictionTime) > 500*time.Millisecond { + cb.EvictFarChunks(TheState.Editor.CursorPosition, 20) + TheState.lastEvictionTime = time.Now() + } } - } */ } @@ -309,11 +304,6 @@ func SetChunkedBuffer(cb *ChunkedBuffer) { TheState.Editor.ChunkedBuffer = cb } -// SetLineIndex sets the line index for the current editor state. -func SetLineIndex(li *types.LineIndex) { - TheState.Editor.LineIndex = li -} - // ToggleSortOrder cycles the browser sort mode through four modes. func ToggleSortOrder(data any) { // Cycle through the 4 sort modes @@ -458,7 +448,7 @@ func HandleEnd() { break } } - + // Position after the last character of the line. // If it's a newline, it's the newline itself. start := layout.ByteOffsets[targetIdx] @@ -842,7 +832,7 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { // Add bottom padding (half line height) so last line isn't flush with the bottom bar. var maxScroll ui.Dp if cb := TheState.Editor.ChunkedBuffer; cb != nil { - if li := TheState.Editor.LineIndex; li != nil { + if li := cb.LineIndex; li != nil { totalLines := li.LineCount() maxScroll = ui.Dp(totalLines)*EditorLineHeight() - editorRegion.H + EditorLineHeight()/2 } else { @@ -915,12 +905,12 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { } // Adjust scroll offset to be relative to visibleContent origin - visibleScrollOffset = ui.Dp(math.Mod(float64(TheState.ScrollOffset),float64(lineHeight))) + visibleScrollOffset = ui.Dp(math.Mod(float64(TheState.ScrollOffset), float64(lineHeight))) // Map scroll offset to a chunk index // Estimate line-to-byte conversion if LineIndex is missing var scrollByteOffset int - if li := TheState.Editor.LineIndex; li != nil { + if li := cb.LineIndex; li != nil { // Precise lineHeight := EditorLineHeight() startLine := int(TheState.ScrollOffset / lineHeight) @@ -931,13 +921,13 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { } } else { // Estimate - scrollByteOffset = int(TheState.ScrollOffset / EditorLineHeight()) * 50 + scrollByteOffset = int(TheState.ScrollOffset/EditorLineHeight()) * 50 } - + scrollChunk := scrollByteOffset / cb.ChunkSize() // Use a smaller radius to prevent overloading the worker pool. // Content() will load chunks immediately needed by the viewport. - cb.Prefetch(scrollChunk, 1) + cb.Prefetch(scrollChunk, 1) } else { fmt.Printf("LAYOUT: fallback, no cb\n") // Fallback: no chunked buffer, use full buffer (small files) @@ -1002,7 +992,7 @@ func SetCursorFromPoint(x, y float64) { // We need to account for scroll offset: y is passed as relative to the text region top + scroll offset. // So y is the position in the *content*. visualLine := int(y / lineHeight) - + log.Printf("SetCursorFromPoint: y=%f, scroll=%f, visualLine=%d, lineHeight=%f", y, float64(TheState.ScrollOffset), visualLine, lineHeight) // Group glyphs by their Y-baseline @@ -1011,16 +1001,16 @@ func SetCursorFromPoint(x, y float64) { indices []int } groups := []lineGroup{} - + // Find all unique baseline Ys // The tap Y is based on line height (top of line). // We need to associate Y-baseline with visual line index. - + // Create map from visual line (0, 1, 2...) to baseline Y. // Since line height is fixed: // Line 0 baseline is at some Y0. // Line 1 baseline is at Y0 + lineHeight. - + // Let's find Y0 first. minY := 1e9 for _, yVal := range layout.Y { @@ -1028,18 +1018,20 @@ func SetCursorFromPoint(x, y float64) { minY = float64(yVal) } } - + // Now group by baseline groups = []lineGroup{} // Reset groups for i, yVal := range layout.Y { yFloat := float64(yVal) - lineIdx := int((yFloat - minY) / lineHeight + 0.5) // round to nearest line - if lineIdx < 0 { lineIdx = 0 } - + lineIdx := int((yFloat-minY)/lineHeight + 0.5) // round to nearest line + if lineIdx < 0 { + lineIdx = 0 + } + // Ensure enough groups for len(groups) <= lineIdx { groups = append(groups, lineGroup{ - y: minY + float64(len(groups))*lineHeight, + y: minY + float64(len(groups))*lineHeight, indices: []int{}, }) }