Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[native_assets_cli] Stop running link hooks in JIT mode. #1252

Closed
3 tasks done
dcharkes opened this issue Jul 2, 2024 · 1 comment
Closed
3 tasks done

[native_assets_cli] Stop running link hooks in JIT mode. #1252

dcharkes opened this issue Jul 2, 2024 · 1 comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_builder package:native_assets_cli

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Jul 2, 2024

We should stop running link hooks for JIT targets.

  1. The hooks can't do anything meaningful over build hooks due to not having treeshaking info ([native_assets_builder] Add hook/link.dart // Tree shaking #153).
  2. Their "map-reduce"-like behavior means that many assets will be touched on rebuilds (cold start JIT, hot reload, hot restart ([native_assets_cli][native_assets_builder] Hot reload and hot restart #1207)).

The price to pay is that in JIT-mode, a build hook needs to do the linking. So likely, there'll be some shared logic between the build and link hooks in packages that have a link hook.

And we need an API for BuildConfig detailing if the link hooks will be run: bool BuildConfig.hasLinkPhase?

  • Add BuildConfig field.
  • Stop running link hooks in JIT mode (Dart and Flutter PRs).
  • Add proper documentation.

Thanks @mkustermann @mosuem @HosseinYousefi @liamappelbe! Please leave additional suggestions, thoughts, etc!

@dcharkes dcharkes added P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_cli package:native_assets_builder labels Jul 2, 2024
@mosuem
Copy link
Member

mosuem commented Jul 3, 2024

Documentation/Messaging on this is important, as we don't want people wondering why their link hooks did or did not run, or even worse, why some assets are there/missing if compiling in JIT vs. AOT.

auto-submit bot pushed a commit that referenced this issue Jul 10, 2024
Another round of rolling so we can update flutter_tools to address:

* #1252
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jul 10, 2024
Note that the `add_asset_link` example project has it's build hook
`exit(nonZero)` because the project only works with link hooks
enabled.
Therefore, the test expectations for `dart run` is that it fails, and
for `dart build` that it succeeds.

Bug: dart-lang/native#1252
Change-Id: I8566d1289a079078586a2e6149f5b77e641618f9
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-arm64-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375100
Commit-Queue: Daco Harkes <[email protected]>
Reviewed-by: Moritz Sümmermann <[email protected]>
auto-submit bot pushed a commit to flutter/flutter that referenced this issue Jul 12, 2024
Stop running link hooks in debug mode.

Rationale: link hooks only get access to tree-shaking info in release builds, so they can't do anything meaningful in debug builds. Debug builds should be fast as development cycle, so running less is better.

More details:

* dart-lang/native#1252

Also: rolls packages to latest versions.

## Implementation details

The decision whether linking is enabled is made as follows:

* For normal builds `build_info.dart::BuildMode` is used to determine whether Dart is compiled in JIT or AOT mode.
* Testers always run in JIT, so no linking.
* Native asset dry runs only run for JIT builds (e.g only when hot reload and hot restart are enabled).

## Testing

The integration test is updated to output an asset for linking if `BuildConfig.linkingEnabled` is true, and to output an asset for bundling directly if linking is not enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_builder package:native_assets_cli
Projects
None yet
Development

No branches or pull requests

2 participants