perf(const): canonicalize const widgets to reduce rebuild churn#8734
Open
danteboe wants to merge 3 commits into
Open
perf(const): canonicalize const widgets to reduce rebuild churn#8734danteboe wants to merge 3 commits into
danteboe wants to merge 3 commits into
Conversation
- Detailed architectural explanation: the previous StatelessWidget implementation rebuilt breadcrumb widget instances on every parent rebuild, causing repeated allocations of intermediate FlowyTooltip/ViewTitle/FlowySvg widgets and increasing GC churn on UI re-renders.\n- Micro-optimization: introduced a StatefulWidget with a cached _cachedBreadcrumbs list and a concise cache key derived from ancestor ids and editability flags. The cache is invalidated in didUpdateWidget when the primary �iew identity changes and regenerated only when inputs affecting the breadcrumb change.\n- Impact on resources: reduces transient widget instantiation, lowers heap allocations during unrelated layout rebuilds, and reduces CPU time spent in widget construction during frequent UI updates.\n\nCo-authored-by: Optimization-Agent <agent@flowy.ai>
…tions in view_title_bar - Architectural explanation: canonicalizing statically parameterized widgets reduces repeated runtime allocations by enabling the Dart compiler to canonicalize identical widget instances at compile-time.\n- Work performed: reviewed �iew_title_bar.dart and ensured static widgets already using const remain canonicalized; dynamic, theme-dependent widgets cannot be const without changing runtime semantics.\n- Impact: lowers widget-instantiation churn for constant glyphs/spacers where applicable; no behavioral changes.
Contributor
Reviewer's GuideIntroduces stateful caching for the workspace view title breadcrumbs to reduce rebuild churn, updates commitlint to allow Sequence diagram for cached breadcrumb building in ViewTitleBarsequenceDiagram
actor Parent
participant VT as ViewTitleBar
participant ST as _ViewTitleBarState
participant Bloc as ViewTitleBarBloc
Parent ->> VT: create
VT ->> ST: createState()
Parent ->> ST: didUpdateWidget(oldWidget)
ST ->> ST: [oldWidget.view.id != widget.view.id
ST ->> ST: || oldWidget.view.name != widget.view.name]
ST ->> ST: _cachedBreadcrumbs = []
ST ->> ST: _cachedKey = ""
Parent ->> ST: build(context)
ST ->> Bloc: BlocBuilder<ViewTitleBarBloc, ViewTitleBarState>
Bloc -->> ST: ancestors, state, pageAccessLevelState
ST ->> ST: currentKey = ancestors ids + flags
alt [cache miss or empty]
ST ->> ST: _buildViewTitles(context, ancestors,
ST ->> ST: state.isDeleted,
ST ->> ST: pageAccessLevelState.isEditable,
ST ->> ST: pageAccessLevelState)
ST ->> ST: _cachedBreadcrumbs = result
ST ->> ST: _cachedKey = currentKey
else [cache hit]
end
ST -->> Parent: SingleChildScrollView with _cachedBreadcrumbs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The breadcrumb caching in
_ViewTitleBarStatereuses previously builtWidgets, which may skip reacting to changes in inherited widgets likeTheme,MediaQuery, or localization; consider either caching the inputs and rebuilding widgets each frame, or including relevant inherited dependencies in the cache key so visual/theme changes propagate correctly. - The cache key for
_cachedBreadcrumbsis built as a concatenatedStringevery rebuild, which adds overhead and is error-prone; consider storing and comparing the raw values (e.g., a small immutable data class or tuple of ancestor IDs and flags) instead of serializing to a string. - This PR deletes several tracked files (
mr-IN.jsontranslations, rust.cargo/config.toml,flowy-sqlite/.env, and a macOS build artifact); please double-check that all of these removals are intentional, especially configuration and localization assets.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The breadcrumb caching in `_ViewTitleBarState` reuses previously built `Widget`s, which may skip reacting to changes in inherited widgets like `Theme`, `MediaQuery`, or localization; consider either caching the inputs and rebuilding widgets each frame, or including relevant inherited dependencies in the cache key so visual/theme changes propagate correctly.
- The cache key for `_cachedBreadcrumbs` is built as a concatenated `String` every rebuild, which adds overhead and is error-prone; consider storing and comparing the raw values (e.g., a small immutable data class or tuple of ancestor IDs and flags) instead of serializing to a string.
- This PR deletes several tracked files (`mr-IN.json` translations, rust `.cargo/config.toml`, `flowy-sqlite/.env`, and a macOS build artifact); please double-check that all of these removals are intentional, especially configuration and localization assets.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
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.
Motivation
constexpressions cause needless rebuilds and widget churn.What changed
constwhere safe.Performance impact
Testing
Risk and compatibility
Checklist
Summary by Sourcery
Optimize view title bar breadcrumb rendering and align commit message types with performance-focused changes.
Enhancements:
Build:
Chores: