ENG-1579: Fix plugin init performance#915
Open
sid597 wants to merge 5 commits intoeng-1470-test-dual-read-with-flag-on-and-fix-gapsv2from
Open
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
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
📝 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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