Skip to content

perf(widgets): use const <Never>{} in InheritedModelElement and avoid redundant casts in RenderObjectElement.update - #192913

Merged
auto-submit[bot] merged 13 commits into
flutter:masterfrom
kevmoo:perf/inherited-model-minimal
Oct 1, 2026
Merged

auto-submit[bot] merged 13 commits into
flutter:masterfrom
kevmoo:perf/inherited-model-minimal

Conversation

@kevmoo

@kevmoo kevmoo commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Description

Two small, self-contained improvements in core widget element update paths that improve generated code and microbenchmark latency across both Native AOT (gen_snapshot x86_64) and WebAssembly (dart2wasm -O2):

  1. InheritedModelElement.updateDependencies (packages/flutter/lib/src/widgets/inherited_model.dart):

    • Avoid HashSet<T>() heap allocation on aspect == null: When a widget subscribes unconditionally to all aspects (e.g., MediaQuery.of(context) or InheritedModel.inheritFrom(context) without an aspect), updateDependencies previously allocated a new mutable HashSet<T>() on the heap for every dependent element solely so dependencies.isEmpty would return true as an "unconditionally subscribed" sentinel. Replacing HashSet<T>() with const <Never>{} replaces a per-dependent heap allocation with a constant pool load (v13 T{_ConstSet} in Dart VM AOT; global.get _ConstSet<Never> in dart2wasm).
    • Avoid redundant generic Set<T> casts and map writes on aspect != null: Typing dependencies as Set<Object?>? and branching on if (dependencies == null) vs else { dependencies.add(aspect); } avoids loading :type_arguments for a parameterized Set<T> check on entry, skips redundant AssertAssignable(aspect, T) checks in release mode when dependencies != null, and avoids re-hashing dependent to overwrite _dependents[dependent] when the set is already stored in the map.
  2. SingleChildRenderObjectElement.update & MultiChildRenderObjectElement.update (packages/flutter/lib/src/widgets/framework.dart):

    • Uses the statically-typed newWidget parameter (newWidget.child and newWidget.children) directly instead of re-reading this.widget (_widget!) and executing runtime casts ((widget as SingleChildRenderObjectWidget).child and widget as MultiChildRenderObjectWidget) immediately after super.update(newWidget).

Performance & Code Generation Verification (Native AOT & WebAssembly)

1. Native AOT (dart compile exe / gen_snapshot x86_64 — 20,000,000 Iterations)

Target Method Pre-Change (Native AOT) Post-Change (Native AOT) Absolute Delta Relative Delta Speedup (vs Pre-Change)
InheritedModelElement.updateDependencies (aspect == null) 39.51 ns/op (790 ms) 26.08 ns/op (521 ms) -13.43 ns/op -34.0% 1.51x faster
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
SingleChildRenderObjectElement.update 5.37 ns/op (107 ms) 4.80 ns/op (95 ms) -0.57 ns/op -10.6% 1.12x faster
  • SingleChildRenderObjectElement.update Native x86_64 Disassembly (79 → 59 instructions, -25.3%):
    • Pre-Change IL: Emits v22 <- LoadField(v2 . _widget) followed by a 20-instruction AssertAssignable(v22, SingleChildRenderObjectWidget) before loading .child.
    • Post-Change IL: Eliminates both LoadField(v2 . _widget) and AssertAssignable, loading v5 <- LoadField(v3 . child) directly from the newWidget register (v3) and reducing machine code from 79 to 59 x86_64 instructions.
  • InheritedModelElement.updateDependencies Native AOT IL:
    • Eliminates LoadField(v2 . :type_arguments) on method entry (as Set<Object?>? uses instantiator_type_args(null) instead of a parameterized Set<T> subtype check).
    • Replaces StaticCall(HashSet.<1> v7) heap allocation with a constant pool reference (v13 T{_ConstSet}) when aspect == null.
    • Eliminates StaticCall([]=<0> _dependents, dependent, dependencies) map write when dependencies != null.

2. WebAssembly (flutter build web --release --wasm, Default -O2)

  • InheritedModelElement.updateDependencies (93 → 91 WAT instructions):
    • Replaces call $_HashSet heap allocation with global.get _ConstSet<Never> when aspect == null, and eliminates _TypeUniverse.isObjectInterfaceSubtype1 + _checkInstance when adding aspects to an existing dependency set.
  • SingleChildRenderObjectElement.update & MultiChildRenderObjectElement.update:
    • Eliminates call $Element.widget and call $"<obj> as SingleChildRenderObjectWidget" / call $"<obj> as MultiChildRenderObjectWidget", reducing MultiChildRenderObjectElement.update (body) from 775 to 767 WAT instructions and reducing sampled CPU hits in MultiChildRenderObjectElement.update by -17.6% (108 → 89 samples) under heavy layout rebuild stress.

Tests

  • Added InheritedModel unconditional dependency sentinel handles aspect transitions in packages/flutter/test/widgets/inherited_model_test.dart.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

…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
@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 17, 2026
@kevmoo
kevmoo marked this pull request as ready for review September 17, 2026 05:17

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread packages/flutter/lib/src/widgets/inherited_model.dart
@kevmoo kevmoo added the CICD Run CI/CD label Sep 17, 2026
Comment thread packages/flutter/lib/src/widgets/inherited_model.dart

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread packages/flutter/lib/src/widgets/inherited_model.dart
Comment thread packages/flutter/test/widgets/inherited_model_test.dart
Comment thread packages/flutter/lib/src/widgets/inherited_model.dart
Comment thread packages/flutter/lib/src/widgets/inherited_model.dart
@LongCatIsLooong

LongCatIsLooong commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

I feel the reason that InheritedModel has to rely on implementation details to avoid dictionary lookup is that the set + get primitives aren't enough. Would it be better if we introduce an https://main-api.flutter.dev/flutter/dart-core/Map/update.html equivalent to the InheritedElement class, so other InheritedWidget subclasses can also enjoy the optimization which avoids the one additional lookup, instead of having to rely on implementation details?

@stuartmorgan-g

Copy link
Copy Markdown
Contributor

test-exempt: code refactor with no semantic change

@kevmoo

kevmoo commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

Would it be better if we introduce an https://main-api.flutter.dev/flutter/dart-core/Map/update.html equivalent to the InheritedElement class, so other InheritedWidget subclasses can also enjoy the optimization which avoids the one additional lookup, instead of having to rely on implementation details?

Good question, @LongCatIsLooong! I benchmarked adding an @pragma('vm:prefer-inline') InheritedElement.updateDependent(dependent, update, {ifAbsent}) (_dependents.update(...)) helper (as well as putIfAbsent) against this PR in Native AOT (x86_64, 40M steady-state rebuild aspect lookups + 2M initial mounts):

Workload (x86_64 Native AOT) PR #192913 (getDependencies + in-place add) InheritedElement Map.update (@pragma('vm:prefer-inline')) InheritedElement Map.putIfAbsent (@pragma('vm:prefer-inline'))
Steady-state rebuild (40M lookups) 801 ms 1,945 ms (2.43x slower) 1,906 ms (2.38x slower)
Fresh mount (2M initial mounts) 213 ms 238 ms (11.7% slower) 234 ms (9.9% slower)

Inspecting the AOT ELF disassembly (objdump -d) and flow graph (--print-flow-graph-optimized) shows why Map.update / putIfAbsent regress performance here:

  1. _HashMap.update / _HashMap.putIfAbsent are not inlined by the Dart VM AOT compiler, forcing heap closure allocations (AllocateContext + AllocateClosure) on every lookup: Even when InheritedElement.updateDependent is annotated @pragma('vm:prefer-inline') and inlined into InheritedModelElement.updateDependencies, _HashMap.update (sdk/lib/_internal/vm_shared/lib/collection_patch.dart:265) and _HashMap.putIfAbsent (line 226) exceed the VM inliner threshold (due to their bucket-chain loop, callback sites, _addEntry, and ArgumentError throw). Because the call to _HashMap.update is not inlined, the update and ifAbsent closures—which must capture this (for generic type parameter T) and aspect—escape into _HashMap.update, emitting call <stub AllocateContext> + call <stub AllocateClosure> on the heap on every updateDependencies call.
  2. Unnecessary bucket writes on steady-state rebuilds: Inside _HashMap.update, the SDK always executes return entry.value = update(unsafeCast<V>(entry.value)) (incurring a map bucket write + GC store barrier + indirect closure call), whereas getDependencies(dependent) followed by dependencies.add(aspect) is a read-only map probe with 0 map bucket writes on steady-state rebuilds.

@LongCatIsLooong LongCatIsLooong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nits, but since this is a performance-only patch, could you add microbenchmarks (especially InheritedModel.updateDependencies)?

Comment thread packages/flutter/lib/src/widgets/inherited_model.dart
chunhtai
chunhtai previously approved these changes Sep 29, 2026

@chunhtai chunhtai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, just some nits

Comment thread packages/flutter/lib/src/widgets/framework.dart Outdated

@LongCatIsLooong LongCatIsLooong left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

https://github.com/search?q=void+setDependencies%28Element+dependent%2C+Object%3F+value%29++lang%3Adart+-path%3Aframework.dart+-path%3Aclass_generics_case1.dart+-path%3Abasic.dart&type=code

There are InheritedElement.setDependencies overrides on github but none of these seems to be InheritedModelElement.setDependencies overrides.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

  1. Make the doc contract explicit: Update InheritedElement.setDependencies (and add a doc comment on InheritedModelElement.updateDependencies) to state directly that subclasses storing a mutable dependency object (like a Set) may mutate the existing object returned by getDependencies in-place inside updateDependencies without calling setDependencies on every update (rather than only implying it in the InheritedModelElement example).
  2. Restore the setDependencies call: Call setDependencies(dependent, dependencies..add(aspect)) in InheritedModelElement.updateDependencies (keeping const <Never>{} and the Set<Object?>? cast cleanup).

@kevmoo kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 1, 2026
Merged via the queue into flutter:master with commit 016ae10 Oct 1, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2026
@kevmoo
kevmoo deleted the perf/inherited-model-minimal branch October 1, 2026 18:27
@kevmoo

kevmoo commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

I slung the autosubmit label too quickly here, @jtmcdole. Doing a follow-up now!

auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Oct 1, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants