From 5fdae1e7d5bb4385d72358956174ee8adf2f5cd2 Mon Sep 17 00:00:00 2001 From: Gregor Lohaus Date: Sun, 31 May 2026 22:16:43 +0200 Subject: [PATCH] avoid redundant copies in render cell marshalling - store codepoints by reference in RenderCell (accessor still clones on read); the constructor clone double-allocated every cell and defeated the shared-empty-array optimization - return rows as an unmodifiable view instead of List.copyOf; the list is a fresh local that never escapes - preallocate row/cell lists to known rows/cols capacity Co-Authored-By: Claude Opus 4.8 --- src/main/java/dev/jlibghostty/RenderCell.java | 5 ++++- .../dev/jlibghostty/internal/GhosttyLibrary.java | 14 +++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/main/java/dev/jlibghostty/RenderCell.java b/src/main/java/dev/jlibghostty/RenderCell.java index 757a62f..68de105 100644 --- a/src/main/java/dev/jlibghostty/RenderCell.java +++ b/src/main/java/dev/jlibghostty/RenderCell.java @@ -22,7 +22,10 @@ public record RenderCell( } public RenderCell { - codepoints = codepoints.clone(); + // Stored by reference: the marshaller hands us freshly allocated (or the shared + // empty) arrays it never mutates, so cloning here would double-allocate every + // cell and defeat the shared-empty optimization. The codepoints() accessor still + // clones on read to protect external callers. foreground = foreground == null ? Optional.empty() : foreground; background = background == null ? Optional.empty() : background; kittyPlaceholder = kittyPlaceholder == null ? Optional.empty() : kittyPlaceholder; diff --git a/src/main/java/dev/jlibghostty/internal/GhosttyLibrary.java b/src/main/java/dev/jlibghostty/internal/GhosttyLibrary.java index dba772e..a2f10a9 100644 --- a/src/main/java/dev/jlibghostty/internal/GhosttyLibrary.java +++ b/src/main/java/dev/jlibghostty/internal/GhosttyLibrary.java @@ -32,6 +32,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Optional; @@ -950,17 +951,20 @@ public final class GhosttyLibrary { try (Arena scratchArena = Arena.ofConfined()) { Scratch scratch = new Scratch(scratchArena); renderStatePopulateRowIterator(state, iterator); - List rows = new ArrayList<>(); + int cols = renderStateGetU16(state, RENDER_STATE_DATA_COLS); + List rows = new ArrayList<>(renderStateGetU16(state, RENDER_STATE_DATA_ROWS)); int rowIndex = 0; while (renderStateRowIteratorNext(iterator)) { boolean dirty = renderStateRowGetBoolean(iterator, RENDER_STATE_ROW_DATA_DIRTY, scratch); List cells = (dirtyRowsOnly && !dirty) ? List.of() - : renderStateRowCells(iterator, scratch); + : renderStateRowCells(iterator, scratch, cols); rows.add(new RenderRow(rowIndex, dirty, cells)); rowIndex++; } - return List.copyOf(rows); + // rows is a fresh local that never escapes, so an unmodifiable wrapper gives + // the same immutability as List.copyOf without re-copying every row per frame. + return Collections.unmodifiableList(rows); } finally { renderStateRowIteratorFree(iterator); } @@ -1054,13 +1058,13 @@ public final class GhosttyLibrary { } } - private List renderStateRowCells(MemorySegment rowIterator, Scratch scratch) { + private List renderStateRowCells(MemorySegment rowIterator, Scratch scratch, int cols) { MemorySegment cells = renderStateRowCellsNew(); try { renderStatePopulateRowCells(rowIterator, cells); // Returned raw: RenderRow's constructor already wraps this in an immutable copy, // so a List.copyOf here would copy the per-cell list a second time every row. - List result = new ArrayList<>(); + List result = new ArrayList<>(cols); int column = 0; while (renderStateRowCellsNext(cells)) { int[] codepoints = renderStateRowCellGraphemes(cells, scratch);