Skip to content

Conversation

@dcharkes
Copy link
Contributor

@dcharkes dcharkes commented May 13, 2025

The link hooks (hook/link.dart) in Flutter have no access to the resource identifier experiment for tree-shaking yet. That means we can start running hooks earlier in the flutter build.

When we do get around to adding tree-shaking information, we should only have a dependency on KernelSnapshot for AOT builds in which the Dart tree-shaker runs. Likely achieved by splitting the DartBuildForNative into two targets, one for Debug and one for Release. (Note, the names for these targets are somewhat weird, so we should address that in another PR as well. It should probably be called RunDartHooks or something.)

I don't expect this PR to have any performance effect for fast hooks. Other targets already run concurrently with this target:

$ flutter build -d macos --verbose
[        ] [ +278 ms] compile_macos_framework: Starting due to {InvalidatedReasonKind.inputChanged: The following inputs have updated contents: /Users/dacoharkes/flt/flutter/examples/hello_world/.dart_tool/flutter_build/ce2ba8b9e7204bce6beba11f20d6df36/app.dill,/Users/dacoharkes/flt/flutter/packages/flutter_tools/lib/src/build_system/targets/macos.dart,/Users/dacoharkes/flt/flutter/bin/cache/engine.stamp}
[...]
[        ] [        ] dart_build: Starting due to {}
[        ] [  +20 ms] No packages with native assets. Skipping native assets compilation.
[        ] [   +1 ms] dart_build: Complete
[        ] [   +1 ms] install_code_assets: Starting due to {}
[        ] [   +1 ms] Writing native assets json to file:///Users/dacoharkes/flt/flutter/examples/hello_world/.dart_tool/flutter_build/ce2ba8b9e7204bce6beba11f20d6df36/native_assets.json.
[        ] [   +1 ms] Writing /Users/dacoharkes/flt/flutter/examples/hello_world/.dart_tool/flutter_build/ce2ba8b9e7204bce6beba11f20d6df36/native_assets.json done.
[        ] [        ] install_code_assets: Complete
[        ] [+1376 ms] Building App.framework for x86_64...
[...]
[        ] [  +41 ms] compile_macos_framework: Complete

However, this PR makes the hooks already run concurrently with kernel compilation, which always takes quite a while.

[        ] [   +2 ms] kernel_snapshot_program: Starting due to {}
[...]
[        ] [+2515 ms] release_unpack_macos: Complete
[        ] [+2425 ms] kernel_snapshot_program: Complete

This means that if the hooks do actual work, and enough cores on the host are available for Dart compilation and hook execution, the result will be quicker than before.

Context

Testing

Existing native assets integration tests

Benchmarks

Currently not benchmarked. This will be benchmarked by the flutter build and flutter-time-to-first-frame benchmarks once the experiment flag is removed.

@dcharkes dcharkes requested a review from goderbauer May 13, 2025 16:58
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@dcharkes dcharkes requested a review from bkonyi May 13, 2025 16:58
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 13, 2025
@goderbauer
Copy link
Member

Is it possible to cover this change with a test (see bot message above)?

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dcharkes
Copy link
Contributor Author

Is it possible to cover this change with a test (see bot message above)?

Added a unit test.

@dcharkes dcharkes added the autosubmit Merge PR when tree becomes green via auto submit App label May 14, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented May 14, 2025

autosubmit label was removed for flutter/flutter/168742, because - The status or check suite Windows build_android_host_app_with_module_aar has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 14, 2025
@dcharkes dcharkes added this pull request to the merge queue May 15, 2025
Merged via the queue into master with commit 3cb2f5f May 15, 2025
149 checks passed
@dcharkes dcharkes deleted the native-assets-earlier branch May 15, 2025 05:44
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 15, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 15, 2025
flutter/flutter@0b9f928...9a78af5

2025-05-15 [email protected] Manual pub package roll (flutter/flutter#168916)
2025-05-15 [email protected] Remove unnecessary `bringup: true` for release-channel only `Linux flutter_packaging`. (flutter/flutter#168761)
2025-05-15 [email protected] Revert: "Run `flutter_packaging` builders on release candidates (flutter/flutter#168917)
2025-05-15 [email protected] Roll Dart SDK from a6c25e31caa7 to c9640c3a4440 (1 revision) (flutter/flutter#168911)
2025-05-15 [email protected] Roll Packages from 1468581 to 2dff621 (4 revisions) (flutter/flutter#168908)
2025-05-15 [email protected] Roll Dart SDK from b3520981e0f0 to a6c25e31caa7 (11 revisions) (flutter/flutter#168895)
2025-05-15 [email protected] Roll Fuchsia Linux SDK from fSvuEJgRmHxnewRJr... to Jj-iDG5uPOsFgY2_H... (flutter/flutter#168893)
2025-05-15 [email protected] Fix mac_ios_engine_ddm config with missing ci/ios_debug_sim_ddm config (flutter/flutter#168888)
2025-05-15 [email protected] [native assets] Remove `KernelSnapshot` dependency in build (flutter/flutter#168742)
2025-05-15 [email protected] iOS,macOS: Migrate logging to Logger/FlutterLogger (flutter/flutter#168568)
2025-05-15 [email protected] Skip hot reload breakpoints test when running with web (flutter/flutter#168873)
2025-05-15 [email protected] CupertinoSliverNavigationBar respects accessibility text scaling (flutter/flutter#168866)
2025-05-15 [email protected] [display_list] paint cleanup. (flutter/flutter#168082)
2025-05-15 [email protected] Add a new CI build for iOS DDM-enabled artifacts (flutter/flutter#168717)
2025-05-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (#168396)" (flutter/flutter#168880)
2025-05-14 [email protected] Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (flutter/flutter#168396)
2025-05-14 [email protected] Mark web_tool_tests_1_2 as bringup. (flutter/flutter#168871)
2025-05-14 [email protected] Marks Mac_mokey run_debug_test_android to be unflaky (flutter/flutter#167634)
2025-05-14 [email protected] Reland "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions" (flutter/flutter#168772)
2025-05-14 [email protected] Remove references to `team-release`. (flutter/flutter#168780)
2025-05-14 [email protected] Make Cupertino sheet set the systemUIStyle through an AnnotatedRegion (flutter/flutter#168182)
2025-05-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use live region in error text input decorator for Android (#165531)" (flutter/flutter#168848)
2025-05-14 [email protected] Normalize BottomAppBarTheme (flutter/flutter#168586)
2025-05-14 [email protected] Roll Packages from 2e166de to 1468581 (2 revisions) (flutter/flutter#168828)
2025-05-14 [email protected] macOS,iOS: fix swift target triple (flutter/flutter#168749)
2025-05-14 [email protected] Further update `Engine-artifacts.md`. (flutter/flutter#168779)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
…r#9259)

flutter/flutter@0b9f928...9a78af5

2025-05-15 [email protected] Manual pub package roll (flutter/flutter#168916)
2025-05-15 [email protected] Remove unnecessary `bringup: true` for release-channel only `Linux flutter_packaging`. (flutter/flutter#168761)
2025-05-15 [email protected] Revert: "Run `flutter_packaging` builders on release candidates (flutter/flutter#168917)
2025-05-15 [email protected] Roll Dart SDK from a6c25e31caa7 to c9640c3a4440 (1 revision) (flutter/flutter#168911)
2025-05-15 [email protected] Roll Packages from 1468581 to 2dff621 (4 revisions) (flutter/flutter#168908)
2025-05-15 [email protected] Roll Dart SDK from b3520981e0f0 to a6c25e31caa7 (11 revisions) (flutter/flutter#168895)
2025-05-15 [email protected] Roll Fuchsia Linux SDK from fSvuEJgRmHxnewRJr... to Jj-iDG5uPOsFgY2_H... (flutter/flutter#168893)
2025-05-15 [email protected] Fix mac_ios_engine_ddm config with missing ci/ios_debug_sim_ddm config (flutter/flutter#168888)
2025-05-15 [email protected] [native assets] Remove `KernelSnapshot` dependency in build (flutter/flutter#168742)
2025-05-15 [email protected] iOS,macOS: Migrate logging to Logger/FlutterLogger (flutter/flutter#168568)
2025-05-15 [email protected] Skip hot reload breakpoints test when running with web (flutter/flutter#168873)
2025-05-15 [email protected] CupertinoSliverNavigationBar respects accessibility text scaling (flutter/flutter#168866)
2025-05-15 [email protected] [display_list] paint cleanup. (flutter/flutter#168082)
2025-05-15 [email protected] Add a new CI build for iOS DDM-enabled artifacts (flutter/flutter#168717)
2025-05-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (#168396)" (flutter/flutter#168880)
2025-05-14 [email protected] Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (flutter/flutter#168396)
2025-05-14 [email protected] Mark web_tool_tests_1_2 as bringup. (flutter/flutter#168871)
2025-05-14 [email protected] Marks Mac_mokey run_debug_test_android to be unflaky (flutter/flutter#167634)
2025-05-14 [email protected] Reland "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions" (flutter/flutter#168772)
2025-05-14 [email protected] Remove references to `team-release`. (flutter/flutter#168780)
2025-05-14 [email protected] Make Cupertino sheet set the systemUIStyle through an AnnotatedRegion (flutter/flutter#168182)
2025-05-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use live region in error text input decorator for Android (#165531)" (flutter/flutter#168848)
2025-05-14 [email protected] Normalize BottomAppBarTheme (flutter/flutter#168586)
2025-05-14 [email protected] Roll Packages from 2e166de to 1468581 (2 revisions) (flutter/flutter#168828)
2025-05-14 [email protected] macOS,iOS: fix swift target triple (flutter/flutter#168749)
2025-05-14 [email protected] Further update `Engine-artifacts.md`. (flutter/flutter#168779)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [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
Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025
…r#9259)

flutter/flutter@0b9f928...9a78af5

2025-05-15 [email protected] Manual pub package roll (flutter/flutter#168916)
2025-05-15 [email protected] Remove unnecessary `bringup: true` for release-channel only `Linux flutter_packaging`. (flutter/flutter#168761)
2025-05-15 [email protected] Revert: "Run `flutter_packaging` builders on release candidates (flutter/flutter#168917)
2025-05-15 [email protected] Roll Dart SDK from a6c25e31caa7 to c9640c3a4440 (1 revision) (flutter/flutter#168911)
2025-05-15 [email protected] Roll Packages from 1468581 to 2dff621 (4 revisions) (flutter/flutter#168908)
2025-05-15 [email protected] Roll Dart SDK from b3520981e0f0 to a6c25e31caa7 (11 revisions) (flutter/flutter#168895)
2025-05-15 [email protected] Roll Fuchsia Linux SDK from fSvuEJgRmHxnewRJr... to Jj-iDG5uPOsFgY2_H... (flutter/flutter#168893)
2025-05-15 [email protected] Fix mac_ios_engine_ddm config with missing ci/ios_debug_sim_ddm config (flutter/flutter#168888)
2025-05-15 [email protected] [native assets] Remove `KernelSnapshot` dependency in build (flutter/flutter#168742)
2025-05-15 [email protected] iOS,macOS: Migrate logging to Logger/FlutterLogger (flutter/flutter#168568)
2025-05-15 [email protected] Skip hot reload breakpoints test when running with web (flutter/flutter#168873)
2025-05-15 [email protected] CupertinoSliverNavigationBar respects accessibility text scaling (flutter/flutter#168866)
2025-05-15 [email protected] [display_list] paint cleanup. (flutter/flutter#168082)
2025-05-15 [email protected] Add a new CI build for iOS DDM-enabled artifacts (flutter/flutter#168717)
2025-05-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (#168396)" (flutter/flutter#168880)
2025-05-14 [email protected] Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (flutter/flutter#168396)
2025-05-14 [email protected] Mark web_tool_tests_1_2 as bringup. (flutter/flutter#168871)
2025-05-14 [email protected] Marks Mac_mokey run_debug_test_android to be unflaky (flutter/flutter#167634)
2025-05-14 [email protected] Reland "Clip search artifacts in CupertinoSliverNavigationBar searchable-to-searchable transitions" (flutter/flutter#168772)
2025-05-14 [email protected] Remove references to `team-release`. (flutter/flutter#168780)
2025-05-14 [email protected] Make Cupertino sheet set the systemUIStyle through an AnnotatedRegion (flutter/flutter#168182)
2025-05-14 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use live region in error text input decorator for Android (#165531)" (flutter/flutter#168848)
2025-05-14 [email protected] Normalize BottomAppBarTheme (flutter/flutter#168586)
2025-05-14 [email protected] Roll Packages from 2e166de to 1468581 (2 revisions) (flutter/flutter#168828)
2025-05-14 [email protected] macOS,iOS: fix swift target triple (flutter/flutter#168749)
2025-05-14 [email protected] Further update `Engine-artifacts.md`. (flutter/flutter#168779)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants