Skip to content

perf(reader): coalesce relocate events and memoize BookCell to stop per-swipe storm - #4562

Merged
chrox merged 2 commits into
readest:mainfrom
loveheaven:perf/coalesce-relocate-memo-bookcell
Jun 12, 2026
Merged

perf(reader): coalesce relocate events and memoize BookCell to stop per-swipe storm#4562
chrox merged 2 commits into
readest:mainfrom
loveheaven:perf/coalesce-relocate-memo-bookcell

Conversation

@loveheaven

Copy link
Copy Markdown
Contributor

Summary

Two complementary changes that stop a swipe burst from re-rendering the full reader tree on every intermediate relocate.

FoliateViewer — coalesce relocate

foliate fires relocate multiple times during a swipe burst (snap steps plus the final stabilize). Each one ended up in setProgress, which writes to readerProgressStore plus bookDataStore. This change coalesces them into a single commit per animation frame via requestAnimationFrame, so only the final viewport state is persisted.

An earlier iteration used requestIdleCallback, but profiling on Android showed "Fire Idle Callback" ballooning to ~2 s of total time per ~28 s session: rIC backed up under sustained pressure and dumped the whole queue into the post-swipe pause, producing exactly the "feels sluggish right after I let go" jank the change was trying to fix. rAF runs once per frame, gets scheduled by the browser's normal vsync loop, and doesn't accumulate when the page is busy.

BooksGrid → BookCell

Previously BooksGrid subscribed to the entire progresses map and rendered all per-book chrome inline, so every page turn re-rendered every cell in the grid (cheap but multiplicative when parallel views are open).

Extract a memoized BookCell component that:

  • subscribes only to its own book's progress via useBookProgress(bookKey) — a page turn re-renders one cell;
  • consumes per-field store selectors so action identities are stable and React.memo doesn't get defeated;
  • receives memoized gridInsets / contentInsets from BooksGrid so memoization on prop identity is meaningful.

Why this matters

Pairs with the progress-store split (#4557). #4557 isolates the high-frequency write source from the wider reader store; this PR makes sure the now-tight reactive surface only fires on full-frame transitions and that only one cell at a time gets the update.

Depends on #4557

This PR's first commit duplicates #4557 (split progress store) so the second commit can compile in isolation (it uses useBookProgress). Once #4557 lands, GitHub will collapse the diff to only the coalesce / BookCell changes. Please merge #4557 first.

Testing

All existing tests pass (pnpm test — 5288 passed).

…er-swipe storm

FoliateViewer
-------------
foliate fires `relocate` multiple times during a swipe burst (snap
steps + intermediate stabilize). Each one ended up in setProgress,
which writes to readerProgressStore + bookDataStore. Coalesce them to
a single commit per animation frame so only the final viewport state
is persisted.

Earlier this used requestIdleCallback, but profiling on Android showed
"Fire Idle Callback" ballooning to 2.0+ s of total time per ~28 s
session: rIC backed up under sustained pressure and dumped the whole
queue into the post-swipe pause, producing exactly the "feels sluggish
right after I let go" jank we were trying to fix. rAF runs once per
frame, gets scheduled by the browser's normal vsync loop, and doesn't
accumulate when the page is busy.

BooksGrid -> BookCell
---------------------
Previously BooksGrid subscribed to the entire progresses map and
rendered every book inline. The map changes on every page turn, so the
whole bookKeys.map(...) body re-ran for every swipe. On top of that
inset-related objects (gridInsets, contentInsets) were rebuilt every
render and threaded as fresh references into 7+ children, so even
unchanged children couldn't bail out. That accounted for ~27% of
main-thread time in the Bottom-Up profile ("Animation Frame Fired"
2.6s / 27%).

Extract BookCell as its own React.memo'd component:
- Each cell subscribes only to its own book's progress via
  useBookProgress(bookKey). A page turn re-renders one BookCell, not
  the grid.
- viewInsets / contentInsets are memoized off their numeric inputs so
  children get stable prop references across renders.
- BookCell uses per-field selectors internally for the same reason
  spelled out in store/readerProgressStore.ts header.
- Dropdown handlers are wrapped in useCallback so HeaderBar's props
  object stays stable.
@loveheaven
loveheaven force-pushed the perf/coalesce-relocate-memo-bookcell branch from 77ddb65 to 74f0226 Compare June 12, 2026 15:19
…on toggles apply live

BookCell subscribed reactively only to useBookProgress and read
viewState/viewSettings imperatively. Settings that save with
applyStyles=false (Show Header/Footer, Double Border, Border Color) and
the bookmark ribbon toggle write no progress, so the cell didn't
re-render and the chrome it gates (SectionInfo, ProgressBar, DoubleBorder,
Ribbon) only updated on the next page turn.

Subscribe to the per-book viewStates[key] slice. This is safe now that
progress lives in its own store — viewStates[key] only bumps on
low-frequency events (settings toggles, ribbon, init, sync), never on
the per-swipe relocate path — so it does not reintroduce the commit
storm the progress-store split removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chrox

chrox commented Jun 12, 2026

Copy link
Copy Markdown
Member

Reviewed and pushed a follow-up fix (714c10a0d).

Issue: the BookCell extraction subscribes reactively only to useBookProgress(bookKey) and reads viewState/viewSettings imperatively. On current main the grid re-rendered on any readerStore change (whole-store destructure subscription), which is what kept the per-book chrome in sync. After this PR, per-book state that doesn't flow through the progress store no longer re-renders the cell:

  • Show Header / Show Footer and Double Border / Border Color save via saveViewSettings(..., applyStyles=false) → no setStyles → no relocate → no progress write.
  • Bookmarking writes setBookmarkRibbonVisibilityviewStates[key] only, no progress.

Since SectionInfo / ProgressBar / Ribbon are mount-gated inside BookCell (and don't self-gate), toggling those left stale chrome until the next page turn.

Fix: BookCell now also subscribes to its own viewStates[bookKey] slice. This is safe precisely because #4557 moved progress into its own store — viewStates[key] now bumps only on low-frequency events (settings toggles, ribbon, init, sync), never on the per-swipe relocate path — so it doesn't reintroduce the commit storm the split removed.

The FoliateViewer rAF coalescing looks correct (stable refs/callback, register-once wiring, unmount flush; previewMode clearing is on the untouched renderer-relocate path).

pnpm test (5288 passed) and pnpm lint (tsgo + Biome) pass.

Minor, left as-is: ReaderContent's handleCloseBook / handleCloseBooksToLibrary aren't useCallback'd, so React.memo(BookCell) is defeated whenever ReaderContent re-renders. It's off the per-page-turn path, and memoizing it correctly also requires stabilizing the throttle(...), so I left it.

@chrox
chrox merged commit 7cba22a into readest:main Jun 12, 2026
7 checks passed
@loveheaven
loveheaven deleted the perf/coalesce-relocate-memo-bookcell branch June 12, 2026 15:59
dalzyu added a commit to dalzyu/readest-hermes that referenced this pull request Jun 18, 2026
chrox added a commit that referenced this pull request Jul 2, 2026
…4900)

Adjusting the top, bottom, left, or right page margin had no visible
effect until an unrelated setting (e.g. Show Header) was toggled.

The BooksGrid perf refactor (#4562) memoized the derived view/content
insets on the ViewSettings object identity. saveViewSettings mutates
ViewSettings in place (same reference), so the memo never recomputed on
a margin edit and the new margin never reached the paginator. Left and
right margins were always stale; top and bottom only refreshed when the
header/footer visibility (an effect dependency) changed, which is why
toggling the header appeared to apply a pending change.

Extract the inset derivation into useContentInsets and memoize by the
resolved numeric values instead of the object reference: identical
numbers across a page turn keep a stable reference (no re-render storm),
while a changed margin yields a new one that propagates to the renderer.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants