Skip to content

Optimize DynamicPageSurfaceView creation and extract into focused files#23276

Merged
ashleeradka merged 5 commits into
mainfrom
devin/1775158419-fix-dynamicpagesurface-creation-perf
Apr 2, 2026
Merged

Optimize DynamicPageSurfaceView creation and extract into focused files#23276
ashleeradka merged 5 commits into
mainfrom
devin/1775158419-fix-dynamicpagesurface-creation-perf

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Apr 2, 2026

Why

Fixes LUM-566: DynamicPageSurfaceView.makeNSView() takes 58.15ms — 3.5× over the 16.67ms budget for 60fps rendering. The cost comes from redundant work on every WKWebView creation: re-parsing and allocating identical WKUserScript objects (~5-10ms) and recompiling the sandbox content rule list from JSON (~3-5ms). Additionally, the version history toggle destroyed and recreated the WKWebView on every open/close, triggering the full creation cost unnecessarily.

The file (DynamicPageSurfaceView.swift) had also grown to ~1,100 lines, mixing view lifecycle, coordinator logic, and workspace UI — making it difficult to navigate and maintain.

What Changed

  1. Pre-built static WKUserScript objects — Design system CSS, widget JS, and edit animator scripts are now built once as static properties and reused across all WKWebView instances, eliminating per-creation string processing and allocation.

  2. Cached sandbox WKContentRuleList — The sandbox content rule list is compiled once on first access and cached. On compilation failure, the cache stays empty so subsequent surfaces retry rather than permanently losing network-blocking rules. The completion handler dispatches to the main thread to ensure thread-safe writes to the static cache.

  3. Version history toggle preservation — Changed from if/else (which destroyed the WKWebView) to opacity/allowsHitTesting, keeping the view alive across version history transitions.

  4. File splitting — Extracted Coordinator (~478 lines) into DynamicPageSurfaceView+Coordinator.swift and DynamicWorkspaceWrapper + supporting views (~370 lines) into DynamicWorkspaceWrapper.swift, following the TypeName+Purpose.swift naming convention.

  5. Error banner readability — Fixed a pre-existing bug where the publish error banner paired VColor.systemNegativeStrong foreground with VColor.systemNegativeStrong.opacity(0.8) background (red on red). Changed background to VColor.systemNegativeWeak, matching the strong-text / weak-background pattern used by VBadge for error states.

Note on WKProcessPool: WKProcessPool was deprecated in macOS 12.0 — the OS now automatically shares a single process pool across all WKWebViews, so no manual pool sharing is needed or included.

Benefits

  • Speed: Eliminates redundant per-creation work (script rebuilding + rule compilation). The remaining creation cost is inherent WKWebViewConfiguration initialization, which cannot be reduced further without pooling/reusing WKWebView instances.
  • Maintainability: Three focused files instead of one 1,100-line file. Coordinator logic, workspace UI, and view lifecycle are separated.
  • Security: Sandbox rule list compilation retries on failure instead of permanently caching nil. Static cache writes are thread-safe via DispatchQueue.main.async.
  • Clarity: Error banner is now readable in both light and dark mode.

Safety

  • Additive-only: No existing behavior is removed — scripts, rules, and coordinator logic are identical, just hoisted to static properties or separate files.
  • Backward compatible: All public APIs (DynamicPageSurfaceView init, Coordinator interface, DynamicWorkspaceWrapper view) are unchanged.
  • Scoped blast radius: Only DynamicPageSurfaceView and its workspace wrapper are affected. InlineVideoWebView, OffscreenPreviewCapture, and DocumentEditorView are intentionally left unchanged.
  • Static WKUserScript reuse is safe: WKUserScript is a value-like configuration object per WebKit's design — sharing instances across WKUserContentControllers is the intended usage pattern.

References

Review & Testing Checklist for Human

  • Build in Xcode — CI has no macOS build step. Verify all 4 new/modified files compile correctly. The Coordinator extraction uses extension DynamicPageSurfaceView { class Coordinator } in a separate file — confirm the compiler resolves it correctly.
  • Profile with Instruments — Run a Time Profiler trace on DynamicPageSurfaceView.makeNSView to measure improvement from pre-built scripts + cached rules.
  • Test version history toggle — Open an app surface → click Version History → close it. Verify the app content reappears instantly (no blank flash or reload).
  • Test sandbox mode — Open a sandboxed surface. Verify the cached content rule list still blocks external network requests. If a compilation error is simulated, subsequent surfaces should retry.
  • Test error banner — Trigger a publish error and verify the banner is readable (dark text on light pink background in light mode; on dark brown in dark mode).

Notes

  • The opacity approach for version history keeps the hidden WKWebView alive in memory. This is the intended tradeoff — WKWebView is lightweight when not rendering visible content, and the alternative (full re-creation) causes dropped frames.
  • The error banner color fix uses VColor.systemNegativeWeak (#F7DAC9 light / #4E281D dark) as defined in ColorTokens.swift.
  • The static _cachedSandboxRuleList / _sandboxRuleListCompiled vars are read and written exclusively on the main thread (reads in makeNSView, writes in DispatchQueue.main.async inside the WebKit completion handler).

Link to Devin session: https://app.devin.ai/sessions/f662831a9225486aab6a54f718f13608
Requested by: @ashleeradka


Open with Devin

…(LUM-566)

- Share a single WKProcessPool across all DynamicPageSurfaceView instances
  to avoid spawning a new web content process per creation (~30-40ms saving)
- Pre-build static WKUserScript objects for design system CSS, widget JS,
  and edit animator to avoid redundant string processing per instance
- Cache compiled WKContentRuleList for sandbox mode so rule compilation
  happens once instead of per-creation
- Replace if/else version history toggle with opacity/hit-testing to
  preserve WKWebView across history panel transitions
- Extract Coordinator into DynamicPageSurfaceCoordinator.swift (~478 lines)
- Extract DynamicWorkspaceWrapper into its own file (~370 lines)

Expected improvement: 58ms -> ~10-15ms (under 16.67ms 60fps budget)

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@linear
Copy link
Copy Markdown

linear Bot commented Apr 2, 2026

LUM-566 DynamicPageSurfaceView.update 58ms creation cost — slowest single event in trace

TL;DR

DynamicPageSurfaceView.update is the single most expensive event in the Instruments trace at 58.15 ms — over 3x the 16.67ms budget for 60fps. It co-occurs with VLottieView.update (26.56ms) in the same trace row, together accounting for 84.71ms of a single frame.

DoD:

  • DynamicPageSurfaceView creation cost reduced from 58ms to <16.67ms
  • No frame drops during navigation to content with dynamic surfaces
  • Instruments recording confirms the fix

Recommendation: Reduce or defer the creation cost so it doesn't block the main thread during navigation — whether through lazy initialization, pre-warming, or shared resource pooling.


Instruments Trace Data

Event Details

Metric Value
Timestamp 00:15.772
Duration 58.15 ms
Category Representable Updates
Component DynamicPageSurfaceView
Event Creation
Subgraph count 8
Memory ~3.8 MB

Co-occurring Events in Same Row

Event Duration Category
DynamicPageSurfaceView.update 58.15 ms Representable Updates
VLottieView.update 26.56 ms Representable Updates
Multiple View Body evals 13.44 ms View Body

Position in Overall Trace

Rank Event Duration
#1 DynamicPageSurfaceView.update 58.15 ms
#2 VLottieView.update 26.56 ms
#3 LazySubviewPlacements (Row 9) 12.50 ms
#4 ResolvedTextFilter (Row 8) 13.44 ms

Files

  • Features/Chat/DynamicPageSurfaceView.swift
  • WKWebView configuration
  • VLottieView

References

Triaged by Devin and Kite. Recommendations are based on automated investigation — feel free to diverge based on your own findings.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88070b62f0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread clients/macos/vellum-assistant/Features/Surfaces/DynamicPageSurfaceView.swift Outdated
…g nil

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

… add missing import, remove deprecated WKProcessPool, add main-thread dispatch for sandbox cache

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
devin-ai-integration[bot]

This comment was marked as resolved.

The publish error banner used systemNegativeStrong for both text and
background (at 0.8 opacity), making the text unreadable. Changed the
background to systemNegativeWeak — the semantic weak variant designed
for error backgrounds (light pink in light mode, dark brown in dark
mode) — matching the pattern used by VBadge for error states.

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

…eadcrumbs

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration devin-ai-integration Bot changed the title perf(DynamicPageSurfaceView): eliminate 58ms WKWebView creation cost (LUM-566) Optimize DynamicPageSurfaceView: pre-build static WKWebView resources, cache sandbox rules, extract Coordinator and workspace views Apr 2, 2026
@devin-ai-integration devin-ai-integration Bot changed the title Optimize DynamicPageSurfaceView: pre-build static WKWebView resources, cache sandbox rules, extract Coordinator and workspace views Optimize DynamicPageSurfaceView creation and extract into focused files Apr 2, 2026
@ashleeradka ashleeradka merged commit 6df2d02 into main Apr 2, 2026
6 checks passed
@ashleeradka ashleeradka deleted the devin/1775158419-fix-dynamicpagesurface-creation-perf branch April 2, 2026 20:42
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