ENG-1579: Fix plugin init performance#915
Conversation
…paths Cache isNewSettingsStoreEnabled(), getFeatureFlags(), getGlobalSettings(), getPersonalSettings(), and all legacy setting readers to avoid redundant Roam DB queries during init. Cache getDiscourseNodes/Relations and fix findDiscourseNode default param that evaluated getDiscourseNodes() before cache check. Add console.time instrumentation for ongoing diagnostics. Init: 27s → 1.1s on megacoglab. Settings panel: 17s → 11ms.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ersion-based invalidation - Remove all console.time instrumentation, replace with single performance.now() total init log - Rename _underscore cache variables to camelCase (drop eslint-disable) - Add configCacheVersion module for cross-module cache staleness without circular imports - Remove redundant invalidateNewSettingsStoreCache (shared watcher path already handles it) - Add TODO(ENG-1471) on legacy dual-read caches - Rename render → openSettingsDialog, measure settings open time via requestAnimationFrame
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThe PR introduces a caching infrastructure for settings and discourse graph structures, with cache invalidation integrated into settings initialization, pull-watch updates, and full config refresh. Performance timing is added to track dialog opening and overall extension startup duration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Hmm. I was really hoping we didn't need to add a cache for all of this. It is not clear why cache is required here.
If this is required just for the dual-read checks on each tab change, then we should look for a different solution to that problem. (eg: manual button click to check).
If this is required for some other reason, I'd like to explore that deeper.
You said
The regression was caused by repeated dual-read validation work during startup
A few questions for this
- Why are we doing dual-read validation on startup?
- How many queries are currently running on startup?
- What, specifically was causing the long load times after init? Eg loading settings and switching tabs?
In any case, we should walk through this together on Monday. Or, feel free to create a loom video walking through the code choices and we can try to do this async.
|
Discussed in 2026-03-30 meeting |
2 participants
The regression was caused by repeated dual-read validation work during startup, not
by block props themselves; we cached that path and fixed invalidation, which brought
init from ~20s down to ~1.15s. This is the right migration-phase fix, and the long-
term cleanup is to delete dual-read entirely.
Summary by CodeRabbit
Release Notes
Refactor
Chores