Add support for JIT release mode #12446

jonahwilliams · 2019-09-25T20:11:30Z

Creates a new Flutter runtime mode "JIT_RELEASE" which implies a product vm but not a precompiled runtime. Adds a new android BuildConfig value of JIT_RELEASE

The macro changes are done. To get this working locally I had to remove a DCHECK on window size (still investigating, possibly a race condition due to slower startup).
Progress towards flutter/flutter#9253

jonahwilliams · 2019-09-25T20:57:39Z

lib/ui/window/viewport_metrics.cc

-  FML_DCHECK(physical_width > 0);
-  FML_DCHECK(physical_height > 0);
-  FML_DCHECK(device_pixel_ratio > 0);
+  // FML_DCHECK(physical_width > 0);


Still working on this one

Due to the slower startup, these seem to be 0 in debug JIT. Is the condition here too strong? Or does this check need to be elided for debug JIT mode?

EDIT: I mean release JIT

jonahwilliams · 2019-09-25T21:11:05Z

cc @chingjun

jason-simmons · 2019-09-25T21:23:12Z

lib/ui/window/viewport_metrics.cc

  // Ensure we don't have nonsensical dimensions.
-  FML_DCHECK(physical_width > 0);
-  FML_DCHECK(physical_height > 0);
+  FML_DCHECK(physical_width >= 0);


Is this related to jit_release mode?

Yes, see #12446 (comment)

I think these checks are just too strong and it should be possible (and often is on release builds) that the viewport metrics dimensions can be zero. Let's just get rid of them.

I've removed all of them

jason-simmons · 2019-09-25T21:45:25Z

runtime/dart_snapshot.cc

-  (OS_WIN || (OS_ANDROID && FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG))
+#define DART_SNAPSHOT_STATIC_LINK                                        \
+  (OS_WIN ||                                                             \
+   (OS_ANDROID && (FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG || \


I think it would be clearer to define a macro that is set whenever the engine is built with a JIT runtime.

It might also be useful to create a macro for modes that are "release modes". I suspect there are other places where the engine currently checks for FLUTTER_RUNTIME_MODE_RELEASE that should be updated to include JIT_RELEASE (e.g. shell/common/switches.cc)

These can be defined by a new header in flutter/common based on the FLUTTER_RUNTIME_MODE set by the build scripts.

Do you have any preference towards a positive/negative approach on these macros? For example PRECOMPILED_RUNTIME (which matches the dart_vm getter) vs JIT_RUNTIME (which calls out the unique behavior) ?

I'd vote for FLUTTER_JIT_RUNTIME and FLUTTER_RELEASE as names for these macros

I've updated them where appropriate to use FLUTTER_RELEASE and FLUTTER_JIT_RUNTIME

chinmaygarde

Please make sure that all engine tests pass when running this variant. ./flutter/testing/run_tests.py --type engine --variant ... before landing this. After landing, also update the LUCI recipes to check the same.

chinmaygarde · 2019-09-25T22:23:22Z

tools/gn

    elif runtime_mode == 'dynamic_release':
        gn_args['dart_runtime_mode'] = 'release'
+    elif runtime_mode == 'jit_release':
+        gn_args['dart_runtime_mode'] = 'release';


jit_release so both variants can in the out directory and engineers tinker on builds locally without one clobbering another.

dart_runtime_mode is used by the Dart SDK. Otherwise, the output directory I have locally is host_jit_release(_unopt) and android_jit_release(_unopt) so this should be okay?

My bad. I misread this line to mean it was the out directory. Thanks for the clarification.

chinmaygarde · 2019-09-25T22:31:25Z

lib/ui/window/viewport_metrics.cc

  // Ensure we don't have nonsensical dimensions.
-  FML_DCHECK(physical_width > 0);
-  FML_DCHECK(physical_height > 0);
+  FML_DCHECK(physical_width >= 0);


I think these checks are just too strong and it should be possible (and often is on release builds) that the viewport metrics dimensions can be zero. Let's just get rid of them.

zanderso · 2019-09-26T15:22:56Z

shell/platform/android/flutter_main.cc

  };

-#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
+#if FLUTTER_JIT_RUNTIME


zanderso · 2019-09-26T15:24:08Z

shell/platform/android/flutter_main.cc

 extern "C" {
-#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
+#if FLUTTER_JIT_RUNTIME
 // Used for debugging dart:* sources.


Is this comment accurate? If it is, then debugging dart:* sources in JIT_RELEASE doesn't seem right.

Ahh you're right. I think I confused this with the application kernel file, but it is just the SDK sources and can be left out

zanderso · 2019-09-26T15:25:43Z

shell/platform/embedder/embedder.cc

 extern "C" {
-#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG
+#if FLUTTER_JIT_RUNTIME
 // Used for debugging dart:* sources.


Same comment here as above.

zanderso · 2019-09-26T15:28:27Z

tools/gn

        gn_args['dart_runtime_mode'] = 'develop'
    elif runtime_mode == 'dynamic_profile':
        gn_args['dart_runtime_mode'] = 'profile'
    elif runtime_mode == 'dynamic_release':


Are 'dynamic_profile' and 'dynamic_release' still used anywhere?

No, I think these could be cleaned up here too if that's what you're asking :)

jonahwilliams · 2019-09-26T16:03:49Z

I have a single failing unit test locally:

[ RUN      ] ShellTest.ReportTimingsIsCalledSoonerInNonReleaseMode
../../flutter/shell/common/shell_unittests.cc:484: Failure
Value of: ellapsed <= fml::TimeDelta::FromMilliseconds(500)
  Actual: false
Expected: true

However, according to the macros it looks like the other test case should be running. I might have configured something wrong, but I'm looking into it now

jonahwilliams · 2019-09-27T15:17:54Z

Update: tests are all passing locally - turns out I messed up the macro definitions! how embarassing

zanderso

lgtm

git@github.com:flutter/engine.git/compare/5b952f286fc0...f0c7740 git log 5b952f2..f0c7740 --no-merges --oneline 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia e242d0ea937c..89e1f600ec78 (3 commits) (flutter/engine#12696) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ncc-K... to KH7Tv... (flutter/engine#12694) 2019-09-30 50856934+nturgut@users.noreply.github.com Refactoring text_editing.dart (flutter/engine#12479) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia abc851eb3dcc..e242d0ea937c (2 commits) (flutter/engine#12693) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0e7500021402..abc851eb3dcc (3 commits) (flutter/engine#12692) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 7aeabcfa6a73..0e7500021402 (1 commits) (flutter/engine#12691) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from eUBos... to sWs8N... (flutter/engine#12690) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from VACNQ... to ncc-K... (flutter/engine#12689) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from OEG20... to eUBos... (flutter/engine#12688) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 6fMRC... to VACNQ... (flutter/engine#12687) 2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia be971636cf9b..7aeabcfa6a73 (3 commits) (flutter/engine#12686) 2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2e856451654d..be971636cf9b (1 commits) (flutter/engine#12685) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from lHiKM... to OEG20... (flutter/engine#12683) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3oH6... to 6fMRC... (flutter/engine#12682) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from nVAcH... to lHiKM... (flutter/engine#12680) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from Kiq-H... to z3oH6... (flutter/engine#12679) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia bd84330ba277..2e856451654d (1 commits) (flutter/engine#12661) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8ab1530cd3cd..bd84330ba277 (1 commits) (flutter/engine#12639) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from qx53U... to nVAcH... (flutter/engine#12620) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from oob8T... to Kiq-H... (flutter/engine#12616) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 28ad6f869822..8ab1530cd3cd (5 commits) (flutter/engine#12614) 2019-09-27 liyuqian@google.com Revert "[fuchsia] Wire up OpacityLayer to Scenic (#11322)" (flutter/engine#12610) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Started asserting the FlutterEngine is running before communicating over channels. (flutter/engine#12469) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Split out the logic to handle status bar touches into its own function (flutter/engine#12587) 2019-09-27 bkonyi@google.com Roll src/third_party/dart c3c31b74fd..132bee48d0 (16 commits) 2019-09-27 iska.kaushik@gmail.com [flutter_runner] Refactor thread_application pair to ActiveApplication (flutter/engine#12573) 2019-09-27 gw280@google.com Reword confusing messaging surrounding unhandled exception in flutter_runner on Fuchsia (flutter/engine#12428) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b23d66e10a98..28ad6f869822 (10 commits) (flutter/engine#12580) 2019-09-27 iska.kaushik@gmail.com Revert "Update linux toolchain for fuchsia" (flutter/engine#12578) 2019-09-27 jonahwilliams@google.com Add support for JIT release mode (flutter/engine#12446) 2019-09-27 iska.kaushik@gmail.com Update linux toolchain for fuchsia (flutter/engine#12572) 2019-09-27 iska.kaushik@gmail.com Remove references to topaz (flutter/engine#12565) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 7901c508c8..c3c31b74fd (4 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3787f51c65c3..b23d66e10a98 (1 commits) (flutter/engine#12559) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from HoRV8... to oob8T... (flutter/engine#12538) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from YDv3O... to qx53U... (flutter/engine#12520) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 296743a86281..3787f51c65c3 (4 commits) (flutter/engine#12512) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 403c4af720..7901c508c8 (7 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b83cc76a5e3a..296743a86281 (1 commits) (flutter/engine#12491) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 327bc451f8..403c4af720 (9 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/clang/mac-amd64 from HfPKR... to zpVtV... (flutter/engine#12473) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370) 2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226) 2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227) ...

git@github.com:flutter/engine.git/compare/5b952f286fc0...f0c7740 git log 5b952f2..f0c7740 --no-merges --oneline 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia e242d0ea937c..89e1f600ec78 (3 commits) (flutter/engine#12696) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from ncc-K... to KH7Tv... (flutter/engine#12694) 2019-09-30 50856934+nturgut@users.noreply.github.com Refactoring text_editing.dart (flutter/engine#12479) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia abc851eb3dcc..e242d0ea937c (2 commits) (flutter/engine#12693) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 0e7500021402..abc851eb3dcc (3 commits) (flutter/engine#12692) 2019-09-30 skia-flutter-autoroll@skia.org Roll src/third_party/skia 7aeabcfa6a73..0e7500021402 (1 commits) (flutter/engine#12691) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from eUBos... to sWs8N... (flutter/engine#12690) 2019-09-30 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from VACNQ... to ncc-K... (flutter/engine#12689) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from OEG20... to eUBos... (flutter/engine#12688) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 6fMRC... to VACNQ... (flutter/engine#12687) 2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia be971636cf9b..7aeabcfa6a73 (3 commits) (flutter/engine#12686) 2019-09-29 skia-flutter-autoroll@skia.org Roll src/third_party/skia 2e856451654d..be971636cf9b (1 commits) (flutter/engine#12685) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from lHiKM... to OEG20... (flutter/engine#12683) 2019-09-29 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from z3oH6... to 6fMRC... (flutter/engine#12682) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from nVAcH... to lHiKM... (flutter/engine#12680) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from Kiq-H... to z3oH6... (flutter/engine#12679) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia bd84330ba277..2e856451654d (1 commits) (flutter/engine#12661) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 8ab1530cd3cd..bd84330ba277 (1 commits) (flutter/engine#12639) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from qx53U... to nVAcH... (flutter/engine#12620) 2019-09-28 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from oob8T... to Kiq-H... (flutter/engine#12616) 2019-09-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia 28ad6f869822..8ab1530cd3cd (5 commits) (flutter/engine#12614) 2019-09-27 liyuqian@google.com Revert "[fuchsia] Wire up OpacityLayer to Scenic (flutter#11322)" (flutter/engine#12610) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Started asserting the FlutterEngine is running before communicating over channels. (flutter/engine#12469) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Split out the logic to handle status bar touches into its own function (flutter/engine#12587) 2019-09-27 bkonyi@google.com Roll src/third_party/dart c3c31b74fd..132bee48d0 (16 commits) 2019-09-27 iska.kaushik@gmail.com [flutter_runner] Refactor thread_application pair to ActiveApplication (flutter/engine#12573) 2019-09-27 gw280@google.com Reword confusing messaging surrounding unhandled exception in flutter_runner on Fuchsia (flutter/engine#12428) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b23d66e10a98..28ad6f869822 (10 commits) (flutter/engine#12580) 2019-09-27 iska.kaushik@gmail.com Revert "Update linux toolchain for fuchsia" (flutter/engine#12578) 2019-09-27 jonahwilliams@google.com Add support for JIT release mode (flutter/engine#12446) 2019-09-27 iska.kaushik@gmail.com Update linux toolchain for fuchsia (flutter/engine#12572) 2019-09-27 iska.kaushik@gmail.com Remove references to topaz (flutter/engine#12565) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 7901c508c8..c3c31b74fd (4 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 3787f51c65c3..b23d66e10a98 (1 commits) (flutter/engine#12559) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from HoRV8... to oob8T... (flutter/engine#12538) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/linux-amd64 from YDv3O... to qx53U... (flutter/engine#12520) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia 296743a86281..3787f51c65c3 (4 commits) (flutter/engine#12512) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 403c4af720..7901c508c8 (7 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia b83cc76a5e3a..296743a86281 (1 commits) (flutter/engine#12491) 2019-09-27 bkonyi@google.com Roll src/third_party/dart 327bc451f8..403c4af720 (9 commits) 2019-09-27 skia-flutter-autoroll@skia.org Roll fuchsia/clang/mac-amd64 from HfPKR... to zpVtV... (flutter/engine#12473) 2019-09-27 skia-flutter-autoroll@skia.org Roll src/third_party/skia ec85f407bfee..b83cc76a5e3a (15 commits) (flutter/engine#12472) 2019-09-27 30870216+gaaclarke@users.noreply.github.com Added a default entrypoint variable to match android syntax. (flutter/engine#12370) 2019-09-26 tauu@h2overclock.de [web_ui] add missing dispose handler for MethodCalls to flutter/platform_view (flutter/engine#12226) 2019-09-26 tauu@h2overclock.de [web_ui] PersistedPlatformView attribute update handling to enable resizing (flutter/engine#12227) ...

add support for JIT release mode

b1a00d9

googlebot added the cla: yes label Sep 25, 2019

jonahwilliams added 2 commits September 25, 2019 13:53

update android build config for JIT release

449cb35

format files

3ba52af

jonahwilliams commented Sep 25, 2019

View reviewed changes

allow zero or greater for metrics dcheck

cdda47e

jonahwilliams changed the title ~~[WIP] Add support for JIT release mode~~ Add support for JIT release mode Sep 25, 2019

jonahwilliams requested review from jason-simmons and zanderso September 25, 2019 21:09

jason-simmons reviewed Sep 25, 2019

View reviewed changes

jason-simmons requested a review from chinmaygarde September 25, 2019 21:49

jonahwilliams mentioned this pull request Sep 25, 2019

Provide x86 (32 and 64-bit?) Android Release binaries flutter/flutter#9253

Closed

chinmaygarde reviewed Sep 25, 2019

View reviewed changes

jonahwilliams added 2 commits September 26, 2019 07:50

switch to new macros

60634bf

fix dart_snapshot

0f41e8a

zanderso reviewed Sep 26, 2019

View reviewed changes

jonahwilliams added 4 commits September 26, 2019 08:40

dont load sdk sources

079445b

update test file

0c24eee

fix comments and remove dynamic from gn

ba8dfe7

update license and more shell unittests

fd4300a

Fix macro definitions

2cc0f1d

jonahwilliams added 4 commits September 27, 2019 08:32

format file

97017e5

preprocessors love parens

58e4001

skip flag test in release mode

fbae38b

format

81a1a98

jonahwilliams requested review from chinmaygarde and zanderso September 27, 2019 16:34

jonahwilliams requested a review from jason-simmons September 27, 2019 16:34

zanderso approved these changes Sep 27, 2019

View reviewed changes

jason-simmons approved these changes Sep 27, 2019

View reviewed changes

jonahwilliams merged commit 6f5eb13 into flutter:master Sep 27, 2019

jonahwilliams deleted the 32_jit_release branch September 27, 2019 18:20

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 30, 2019

6f5eb13 Add support for JIT release mode (flutter/engine#12446)

dab93d1

engine-flutter-autoroll mentioned this pull request Sep 30, 2019

Roll engine 5b952f286fc0..f0c77405d5c2 (47 commits) flutter/flutter#41622

Merged

Add support for JIT release mode #12446

Add support for JIT release mode #12446

Uh oh!

Conversation

jonahwilliams commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Sep 26, 2019

Uh oh!

jonahwilliams commented Sep 27, 2019

Uh oh!

zanderso left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonahwilliams commented Sep 25, 2019 •

edited

Loading