Add an option to use a prebuilt Dart SDK #26931

zanderso · 2021-06-24T05:27:56Z

This PR downloads prebuilt Dart SDKs into a new prebuilts/ directory that match the version of the Dart SDK source that is fetched through the DEPS file.

During a build, these prebuilts are copied to e.g. out/host_debug_unopt/dart-sdk instead of building them. This is faster than building the snapshots for dart2js, etc..

$ ./flutter/tools/gn --unoptimized --no-lto --goma --full-dart-sdk --prebuilt-dart-sdk
...
$ time ninja -C out/host_debug_unopt -j200
[4516/4516] STAMP obj/default.stamp
...
real	1m23.145s
user	6m15.124s
sys	1m5.422s

$ ./flutter/tools/gn --unoptimized --no-lto --goma --full-dart-sdk
...
$ time ninja -C out/host_debug_unopt -j200
[6051/6051] STAMP obj/default.stamp

real	2m37.590s
user	18m23.790s
sys	2m38.786s

This patch needs https://dart-review.googlesource.com/c/sdk/+/204760.

…ctory This is needed for the Flutter Engine to use a prebuilt Dart SDK as in flutter/engine#26931. When using a prebuilt Dart SDK, the rules producing the built SDK are directed to a different location so that GN doesn't warn about multiple rules that produce the same file. TEST=locally with a flutter/engine checkout Change-Id: I4095f379c772c63f93a1c88c7f4e44804f3a6fc7 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204760 Reviewed-by: Alexander Aprelev <aam@google.com> Commit-Queue: Zach Anderson <zra@google.com>

flutter-dashboard · 2021-07-07T21:49:54Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

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.

dnfield · 2021-07-07T22:44:42Z

DEPS

+  {
+    'name': 'Download prebuilt Dart SDK',


Should we guard this with some kind of variable? Will downloading this always be necessary - in particular, will there be any builders that still use --full-dart-sdk and so should skip this?

Good idea. The API in the recipes appears to use env vars, so I added a guard on one in the python script. Next step is to enable in recipes and see how much things get better there.

chinmaygarde

I am a bit nervous about missing something that the CIPD hook already does for us (curl on host, hash checks, fixing permissions, etc) besides just having to maintain another script. But, I couldn't spot anything wrong with this approach till the artifacts are published on CIPD. Is this something that is being tracked?

chinmaygarde · 2021-07-07T22:43:18Z

web_sdk/BUILD.gn

-    "//third_party/dart/utils/dartdevc:dartdevc_files_stamp",
-    "//third_party/dart/utils/dartdevc:dartdevc_sdk_patch_stamp",
-  ]
+template("_dartdevc") {


I don't know what these templates are supposed to do and neither are their arguments documented. It's possible to infer both after reading the implementation but a short docstring might be nice.

Added comments.

chinmaygarde · 2021-07-07T22:46:44Z

tools/download_dart_sdk.py

+# Downloads a Dart SDK to //flutter/prebuilts.
+def DownloadDartSDK(channel, version, os_name, arch):
+  file = 'dartsdk-{}-{}-release.zip'.format(os_name, arch)
+  url = 'https://storage.googleapis.com/dart-archive/channels/{}/raw/{}/sdk/{}'.format(


This seems to be recreating the functionality of CIPD hooks in the DEPS file. Can we move these artifacts to CIPD instead? Stuff like needing curl on the host and not performing hash consistency checks may lead to unnecessary hassles on CI.

Yes. Sorry for not being more explicit about the plan. I want to see how much this can improve build times on CI. If there's a reasonably good improvement, then I'll open a thread with Dart EngProd to request that these artifacts are uploaded to cipd, and this script will go away.

If the improvement on CI is not significant, then I will back all this out.

chinmaygarde · 2021-07-07T22:50:07Z

tools/download_dart_sdk.py

+  # Download and extract variants in parallel
+  pool = multiprocessing.Pool()
+  tasks = [(channel, semantic_version, os_name, arch) for arch in architectures]
+  async_results = [pool.apply_async(DownloadAndExtract, t) for t in tasks]


Don't thank me, thank stackoverflow!

zanderso added the Work in progress (WIP) Not ready (yet) for review! label Jun 24, 2021

flutter-dashboard bot added the platform-web Code specifically for the web engine label Jun 24, 2021

google-cla bot added the cla: yes label Jun 24, 2021

zanderso force-pushed the prebuilt-dart-sdk branch from 3d7ba77 to c5e44b0 Compare June 24, 2021 05:56

zanderso mentioned this pull request Jun 24, 2021

[engine] snapshot generation takes a long time, making builds slow flutter/flutter#84787

Closed

zanderso force-pushed the prebuilt-dart-sdk branch 8 times, most recently from b080f27 to 555964c Compare June 29, 2021 03:11

zanderso force-pushed the prebuilt-dart-sdk branch 3 times, most recently from 83027b1 to c387624 Compare July 7, 2021 21:35

flutter-dashboard bot added the needs tests label Jul 7, 2021

zanderso removed the Work in progress (WIP) Not ready (yet) for review! label Jul 7, 2021

zanderso requested review from yjbanov, chinmaygarde and dnfield July 7, 2021 22:39

dnfield reviewed Jul 7, 2021

View reviewed changes

chinmaygarde approved these changes Jul 7, 2021

View reviewed changes

Add an option to use a prebuilt Dart SDK

51061c6

zanderso force-pushed the prebuilt-dart-sdk branch from c387624 to 51061c6 Compare July 8, 2021 16:51

zanderso merged commit 9aa280e into flutter:master Jul 8, 2021

zanderso deleted the prebuilt-dart-sdk branch July 8, 2021 17:36

engine-flutter-autoroll mentioned this pull request Jul 8, 2021

Roll Engine from 6f00ff26d767 to 9aa280e35589 (4 revisions) flutter/flutter#86115

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 8, 2021

9aa280e Add an option to use a prebuilt Dart SDK (flutter/engine#26931)

361c097

moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021

Add an option to use a prebuilt Dart SDK (flutter#26931)

431eae9

naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021

Add an option to use a prebuilt Dart SDK (flutter#26931)

e64f2ad

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an option to use a prebuilt Dart SDK #26931

Add an option to use a prebuilt Dart SDK #26931

zanderso commented Jun 24, 2021 •

edited

Loading

flutter-dashboard bot commented Jul 7, 2021

dnfield Jul 7, 2021

zanderso Jul 8, 2021

chinmaygarde left a comment

chinmaygarde Jul 7, 2021

zanderso Jul 8, 2021

chinmaygarde Jul 7, 2021

zanderso Jul 8, 2021 •

edited

Loading

chinmaygarde Jul 7, 2021

zanderso Jul 8, 2021

Add an option to use a prebuilt Dart SDK #26931

Add an option to use a prebuilt Dart SDK #26931

Conversation

zanderso commented Jun 24, 2021 • edited Loading

flutter-dashboard bot commented Jul 7, 2021

dnfield Jul 7, 2021

Choose a reason for hiding this comment

zanderso Jul 8, 2021

Choose a reason for hiding this comment

chinmaygarde left a comment

Choose a reason for hiding this comment

chinmaygarde Jul 7, 2021

Choose a reason for hiding this comment

zanderso Jul 8, 2021

Choose a reason for hiding this comment

chinmaygarde Jul 7, 2021

Choose a reason for hiding this comment

zanderso Jul 8, 2021 • edited Loading

Choose a reason for hiding this comment

chinmaygarde Jul 7, 2021

Choose a reason for hiding this comment

zanderso Jul 8, 2021

Choose a reason for hiding this comment

zanderso commented Jun 24, 2021 •

edited

Loading

zanderso Jul 8, 2021 •

edited

Loading