Repository navigation
perf(widgets): use const <Never>{} in InheritedModelElement and avoid redundant casts in RenderObjectElement.update - #192913
Conversation
…onentElement.performRebuild - Skip redundant ancestor.updateDependencies(this, null) calls in Element.dependOnInheritedElement on subsequent rebuilds when the ancestor does not track aspect dependencies (InheritedElement.hasAspectDependencies == false). - Replace per-dependent HashSet<T>() allocation for unconditional dependencies (aspect == null) in InheritedModelElement.updateDependencies with a shared const Set<Never>, and avoid redundant generic covariant subtype checks against T and redundant setDependencies calls. - Outline cold-path ErrorWidget/FlutterError exception handling in ComponentElement.performRebuild into @pragma('vm:never-inline') @pragma('wasm:never-inline') helper methods and move super.performRebuild() out of the outer finally block to eliminate nested Wasm try/rethrow exception tables.
…ctElement.update and direct clamping in Priority.+
… redundant casts in RenderObjectElement.update
There was a problem hiding this comment.
Code Review
This pull request refactors SingleChildRenderObjectElement and MultiChildRenderObjectElement to use newWidget directly, and updates InheritedModelElement.updateDependencies to optimize dependency tracking by using a constant empty set for null aspects and simplifying dependency additions. A new test case is also added to verify aspect transitions. Feedback suggests simplifying the control flow in updateDependencies by returning early when aspect is null, ensuring that the assert(aspect is T) check is consistently executed for all non-null aspects.
…in updateDependencies
chunhtai
left a comment
There was a problem hiding this comment.
There is no regression test, but the change doesn't seem testable to me. should reach out the text exemption approver.
Otherwise, the change lgtm though
|
I feel the reason that |
|
test-exempt: code refactor with no semantic change |
Good question, @LongCatIsLooong! I benchmarked adding an
Inspecting the AOT ELF disassembly (
|
LongCatIsLooong
left a comment
There was a problem hiding this comment.
LGTM with nits, but since this is a performance-only patch, could you add microbenchmarks (especially InheritedModel.updateDependencies)?
…tDependencies and refine microbenchmark
80f052a
There was a problem hiding this comment.
I'm still a bit unsure about the skipping setDependencies part. Would it be ok to move this to a different PR?
| InheritedModelElement.updateDependencies (aspect != null, existing Set) | 33.75 ns/op (675 ms) | 22.51 ns/op (450 ms) | -11.24 ns/op | -33.3% | 1.50x faster |
|---|
For an app that has 100 MediaQuery.Xof and Theme.Xof calls in its rebuild path -11.24 ns/op is like 0.006744% of the 60 fps frame budget. It's not negligible (and maybe the percentage is higher on a mobile device) but I'm not sure it's a good tradeoff (over losing the ability to allow InheritedModelElement.setDependencies overriding).
| /// so that they can selectively rebuild dependents in [notifyDependent]. | ||
| /// | ||
| /// This method is typically only called in overrides of [updateDependencies]. | ||
| /// For example, [InheritedModelElement.updateDependencies] calls this method |
There was a problem hiding this comment.
If we make the promise in API docs (that it is ok to modify the aspect object in-place for performance reasons and subclasses shouldn't expect setDependencies to always be called when the aspect object of a dependent changes) then I think we should make a clear statement instead of implying that in an example.
Also I think I previously overlooked the case where clients override InheritedModelElement.setDependencies to add side effects and expect that to always be called in updateDependencies. It looks like this performance change can affect the behavior of apps.
There was a problem hiding this comment.
There are InheritedElement.setDependencies overrides on github but none of these seems to be InheritedModelElement.setDependencies overrides.
There was a problem hiding this comment.
auto-submit merged this before I saw your final comments
I also checked google3 and confirmed what your GitHub search found: there are 0 subclasses of InheritedModelElement and 0 external overrides of setDependencies internally as well.
I'm happy to send a quick follow-up PR Either way—which would you prefer?
- Make the doc contract explicit: Update
InheritedElement.setDependencies(and add a doc comment onInheritedModelElement.updateDependencies) to state directly that subclasses storing a mutable dependency object (like aSet) may mutate the existing object returned bygetDependenciesin-place insideupdateDependencieswithout callingsetDependencieson every update (rather than only implying it in theInheritedModelElementexample). - Restore the
setDependenciescall: CallsetDependencies(dependent, dependencies..add(aspect))inInheritedModelElement.updateDependencies(keepingconst <Never>{}and theSet<Object?>?cast cleanup).
|
I slung the |
…13104) Manual roll requested by [email protected] flutter/flutter@d649d2b...e89fd0a 2026-10-01 [email protected] Manual roll ICU from 6ebb40c59477 to 5aa526207171 (2 revisions) (flutter/flutter#193661) 2026-10-01 [email protected] fix(web): hash source maps before rewriting sourceMappingURL with --web-content-hash (flutter/flutter#193614) 2026-10-01 [email protected] [flutter_tools] Bootstrap DeviceManager in ToolDependencies (flutter/flutter#193524) 2026-10-01 [email protected] [wasm] Set flutter_runtime_mode in tools/gn to_gn_wasm_args (flutter/flutter#193407) 2026-10-01 [email protected] Remove remaining `download_jdk` references (flutter/flutter#193562) 2026-10-01 [email protected] Synchronize dart2wasm feature probes in roll script and flutter_bootstrap.js (flutter/flutter#193271) 2026-10-01 [email protected] [web] Unskip web tests that now pass in gestures, services, and widgets (flutter/flutter#193512) 2026-10-01 [email protected] Roll Fuchsia Test Scripts from whZAV1mfT-Rdc0rQ8... to fCNknap4ZI0AMcvIf... (flutter/flutter#193640) 2026-10-01 [email protected] Roll Skia from 0575a37d0619 to f2d68e0b8863 (2 revisions) (flutter/flutter#193621) 2026-10-01 [email protected] perf(widgets): use const <Never>{} in InheritedModelElement and avoid redundant casts in RenderObjectElement.update (flutter/flutter#192913) 2026-10-01 [email protected] [web] Unskip and strengthen missing glyph tofu test on Wimp (flutter/flutter#193426) 2026-10-01 [email protected] [flutter_tools] Fix `flutter drive -d chrome` hanging on web (flutter/flutter#193459) 2026-10-01 [email protected] [web] Fix hang when Chromium cannot bind remote debugging port (flutter/flutter#193461) 2026-10-01 [email protected] [flutter_tools] Bundle data assets from hooks in flutter test (flutter/flutter#193435) 2026-10-01 [email protected] Roll Packages from 0ba9a82 to d5ec6db (11 revisions) (flutter/flutter#193641) 2026-10-01 [email protected] Revert: [flutter_tools] Fix DTD "Stream already subscribed" error and wait for analysis over DTD in widget preview (flutter/flutter#193637) 2026-10-01 [email protected] Roll Skia from 6182a3cf8f98 to 0575a37d0619 (5 revisions) (flutter/flutter#193613) 2026-10-01 [email protected] [flutter_tools] Fix DTD "Stream already subscribed" error and wait for analysis over DTD in widget preview (flutter/flutter#192930) 2026-09-30 [email protected] [Impeller] Sample the render target rect in offscreen advanced blends (flutter/flutter#192988) 2026-09-30 [email protected] Roll Skia from 01524014dff5 to 6182a3cf8f98 (11 revisions) (flutter/flutter#193594) 2026-09-30 [email protected] [web] Use Xvfb -displayfd and clean up Xvfb after Firefox test suites (flutter/flutter#193510) 2026-09-30 [email protected] Fixes bug where sibling node fail to update size (flutter/flutter#191220) 2026-09-30 [email protected] Roll Skia from c7b323126bf0 to 01524014dff5 (3 revisions) (flutter/flutter#193577) 2026-09-30 [email protected] Remove some Undefined Behavior from run_customer_testing_tests.dart. (flutter/flutter#193582) 2026-09-30 [email protected] Fix pointer id/coordinate mismatch in Android platform view multi-touch (flutter/flutter#191631) 2026-09-30 [email protected] Roll Packages from 3c6ce59 to 0ba9a82 (11 revisions) (flutter/flutter#193578) 2026-09-30 [email protected] Fix unresolved doc comment references in dev/ tooling (flutter/flutter#193501) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Description
Two small, self-contained improvements in core widget element update paths that improve generated code and microbenchmark latency across both Native AOT (
gen_snapshotx86_64) and WebAssembly (dart2wasm-O2):InheritedModelElement.updateDependencies(packages/flutter/lib/src/widgets/inherited_model.dart):HashSet<T>()heap allocation onaspect == null: When a widget subscribes unconditionally to all aspects (e.g.,MediaQuery.of(context)orInheritedModel.inheritFrom(context)without an aspect),updateDependenciespreviously allocated a new mutableHashSet<T>()on the heap for every dependent element solely sodependencies.isEmptywould returntrueas an "unconditionally subscribed" sentinel. ReplacingHashSet<T>()withconst <Never>{}replaces a per-dependent heap allocation with a constant pool load (v13 T{_ConstSet}in Dart VM AOT;global.get _ConstSet<Never>indart2wasm).Set<T>casts and map writes onaspect != null: TypingdependenciesasSet<Object?>?and branching onif (dependencies == null)vselse { dependencies.add(aspect); }avoids loading:type_argumentsfor a parameterizedSet<T>check on entry, skips redundantAssertAssignable(aspect, T)checks in release mode whendependencies != null, and avoids re-hashingdependentto overwrite_dependents[dependent]when the set is already stored in the map.SingleChildRenderObjectElement.update&MultiChildRenderObjectElement.update(packages/flutter/lib/src/widgets/framework.dart):newWidgetparameter (newWidget.childandnewWidget.children) directly instead of re-readingthis.widget(_widget!) and executing runtime casts ((widget as SingleChildRenderObjectWidget).childandwidget as MultiChildRenderObjectWidget) immediately aftersuper.update(newWidget).Performance & Code Generation Verification (Native AOT & WebAssembly)
1. Native AOT (
dart compile exe/gen_snapshotx86_64 —20,000,000Iterations)InheritedModelElement.updateDependencies (aspect == null)39.51 ns/op(790 ms)26.08 ns/op(521 ms)-13.43 ns/op-34.0%1.51xfasterInheritedModelElement.updateDependencies (aspect != null, existing Set)33.75 ns/op(675 ms)22.51 ns/op(450 ms)-11.24 ns/op-33.3%1.50xfasterSingleChildRenderObjectElement.update5.37 ns/op(107 ms)4.80 ns/op(95 ms)-0.57 ns/op-10.6%1.12xfasterSingleChildRenderObjectElement.updateNative x86_64 Disassembly (79→59instructions,-25.3%):v22 <- LoadField(v2 . _widget)followed by a 20-instructionAssertAssignable(v22, SingleChildRenderObjectWidget)before loading.child.LoadField(v2 . _widget)andAssertAssignable, loadingv5 <- LoadField(v3 . child)directly from thenewWidgetregister (v3) and reducing machine code from 79 to 59 x86_64 instructions.InheritedModelElement.updateDependenciesNative AOT IL:LoadField(v2 . :type_arguments)on method entry (as Set<Object?>?usesinstantiator_type_args(null)instead of a parameterizedSet<T>subtype check).StaticCall(HashSet.<1> v7)heap allocation with a constant pool reference (v13 T{_ConstSet}) whenaspect == null.StaticCall([]=<0> _dependents, dependent, dependencies)map write whendependencies != null.2. WebAssembly (
flutter build web --release --wasm, Default-O2)InheritedModelElement.updateDependencies(93→91WAT instructions):call $_HashSetheap allocation withglobal.get _ConstSet<Never>whenaspect == null, and eliminates_TypeUniverse.isObjectInterfaceSubtype1+_checkInstancewhen adding aspects to an existing dependency set.SingleChildRenderObjectElement.update&MultiChildRenderObjectElement.update:call $Element.widgetandcall $"<obj> as SingleChildRenderObjectWidget"/call $"<obj> as MultiChildRenderObjectWidget", reducingMultiChildRenderObjectElement.update (body)from775to767WAT instructions and reducing sampled CPU hits inMultiChildRenderObjectElement.updateby-17.6%(108→89samples) under heavy layout rebuild stress.Tests
InheritedModel unconditional dependency sentinel handles aspect transitionsinpackages/flutter/test/widgets/inherited_model_test.dart.Pre-launch Checklist
///).