Skip to content

perf(diff): reuse evicted worker highlights - #791

Merged
benvinegar merged 2 commits into
mainfrom
experiment/worker-highlight-cache
Aug 17, 2026
Merged

benvinegar merged 2 commits into
mainfrom
experiment/worker-highlight-cache

Conversation

@benvinegar

@benvinegar benvinegar commented Aug 17, 2026 •

Copy link
Copy Markdown
Member

Summary

  • retain compact syntax-highlight payloads in Hunk's existing Bun worker under an 8 MiB byte-bounded LRU
  • safely clone cached typed-array buffers before transferring responses to the terminal process
  • add content-addressed worker cache identity, coverage, and focused cache-layer benchmarks

Validation

  • bun run format:check
  • bun run lint
  • bun run typecheck
  • bun test src/ui/diff/diffRows.test.ts src/ui/diff/worker/highlightCompact.test.ts src/ui/diff/worker/highlightWorkerCache.test.ts src/ui/diff/worker/highlightWorkerIdentity.test.ts src/ui/diff/worker/highlightWorkerClient.test.ts
  • bun run bench:highlight-cache-layers

v0.19.0 comparison

Seven fresh processes each highlighted the same 8,000-line TypeScript diff, then made five sequential identical worker requests.

Metric (median) v0.19.0 This PR
Cold request 374.29 ms 380.73 ms
Repeated direct worker request 234.65 ms 1.48 ms

The worker cache makes repeated direct worker requests about 159× faster in this controlled revisit workload. Cold performance is effectively unchanged.

bench:highlight-cache-layers also tests the real prefetchHighlightedDiff path: it makes eight unique 8,000-line requests (exceeding the 60k-line main cache) and revisits the first. Across five fresh-process samples, the resident main-cache hit remained 0.32 ms in both revisions; the post-eviction revisit fell from 233.24 ms on v0.19.0 to 3.25 ms on this PR. The main cache remains worthwhile: it is about 10× faster than a worker-LRU hit and supplies a highlighted snapshot during render instead of waiting for an effect.

bun test was also attempted; its three failures are pre-existing missing website test dependencies (@axe-core/playwright and @playwright/test) in this worktree.

This PR description was generated by Pi using gpt-5.6-terra

@vercel

vercel Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 17, 2026 3:08am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds a worker-local, byte-bounded LRU so syntax-highlight results can be reused after eviction from the terminal-side cache. It also introduces content-derived cache identities, transferable payload cloning, focused tests, and a cold-versus-warm benchmark.

  • Hashes all worker request inputs to identify reusable highlights.
  • Retains compact payloads under an 8 MiB LRU and transfers cloned buffers.
  • Adds cache ownership, eviction, identity, and benchmark coverage.

Confidence Score: 4/5

The PR appears safe to merge, with only a non-blocking formatting inconsistency in the newly added TypeScript.

The cache identity, serialized request lifecycle, clone-before-transfer ownership, and eviction behavior preserve current highlighting correctness; the remaining accepted issue is limited to formatting consistency.

Files Needing Attention: src/ui/diff/worker/highlightWorkerCache.ts and other newly added TypeScript files

Important Files Changed

Filename Overview
src/ui/diff/worker/highlightWorker.ts Integrates content-addressed cache lookup, rendering on misses, and safe transfer of cloned cached payloads.
src/ui/diff/worker/highlightWorkerCache.ts Adds the byte-bounded LRU implementation; behavior appears sound, but its added formatting violates an applicable repository instruction.
src/ui/diff/worker/highlightWorkerIdentity.ts Hashes the complete worker request identity with a revision marker; no reachable identity collision was established.
src/ui/diff/worker/highlightCompact.ts Adds complete deep cloning of compact payload arrays and retained-size estimation.
src/ui/diff/diffRows.ts Refactors worker request inputs into named local values without changing request semantics.
src/ui/diff/worker/highlightWorkerCache.test.ts Covers transfer ownership, LRU eviction, oversized payload rejection, and replacement accounting.

Sequence Diagram

sequenceDiagram
    participant T as Terminal
    participant W as Highlight worker
    participant C as Worker LRU
    T->>W: Highlight request
    W->>W: Hash rendering inputs
    W->>C: Lookup cache key
    alt Cache hit
        C-->>W: Clone retained compact payload
    else Cache miss
        W->>W: Render and compact highlight
        W->>C: Retain payload if within budget
        W->>W: Clone retained payload
    end
    W-->>T: Transfer response buffers
Loading
Prompt To Fix All With AI
### Issue 1
src/ui/diff/worker/highlightWorkerCache.ts:21-24
**Use prescribed TypeScript indentation**

The newly added worker-cache implementation uses two-space indentation instead of the applicable four-space convention, leaving the new TypeScript inconsistent with the repository's prescribed formatting.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "perf(diff): reuse evicted worker highlig..." | Re-trigger Greptile

Comment on lines +21 to +24
constructor(maxBytes = MAX_WORKER_HIGHLIGHT_CACHE_BYTES) {
this.maxBytes = Number.isFinite(maxBytes) ? Math.max(1, Math.floor(maxBytes)) : 1;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Use prescribed TypeScript indentation

The newly added worker-cache implementation uses two-space indentation instead of the applicable four-space convention, leaving the new TypeScript inconsistent with the repository's prescribed formatting.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/diff/worker/highlightWorkerCache.ts
Line: 21-24

Comment:
**Use prescribed TypeScript indentation**

The newly added worker-cache implementation uses two-space indentation instead of the applicable four-space convention, leaving the new TypeScript inconsistent with the repository's prescribed formatting.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar
benvinegar force-pushed the experiment/worker-highlight-cache branch from c5554be to cbf2fc3 Compare August 17, 2026 03:08
@benvinegar
benvinegar merged commit 6c8cacf into main Aug 17, 2026
12 checks passed
benvinegar pushed a commit that referenced this pull request Aug 17, 2026
Merges main, whose new cache-layer benchmark (#791) landed importing the
core/types shell that this branch melts; DiffFile now comes from its
declaring module.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcuayyxJrwQd9EHpuxNef1
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.

1 participant