Build a specialized snapshot for launching the service isolate in profile mode on Android #29245
Conversation
|
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. |
common/settings.h
Outdated
| @@ -116,6 +116,10 @@ struct Settings { | |||
| // case the primary path to the library can not be loaded. | |||
| std::vector<std::string> application_library_path; | |||
|
|
|||
| // Path to a library containing compiled Dart code usable for launching | |||
| // the VM service isolate. | |||
| std::string vmservice_snapshot_library_path; | |||
There was a problem hiding this comment.
Should this be similar to the application_library_path where a vector of paths is specified?
| @@ -134,7 +134,12 @@ template("flutter_snapshot") { | |||
| "--assembly=" + rebase_path(snapshot_assembly), | |||
| ] | |||
| } else if (is_android) { | |||
| libapp = "$target_gen_dir/android/libs/$android_app_abi/libapp.so" | |||
| if (defined(invoker.output_aot_lib)) { | |||
There was a problem hiding this comment.
output_aot_lib is a bit obscure and undocumented. Can we make this an required argument (and update existing callers) and add an assertion at the top of this template saying what this is meant to do.
There was a problem hiding this comment.
Added documentation. I'd prefer to keep this parameter optional - in all other cases callers will be building conventional app snapshots and will want to use the default filename expected by the embedding.
|
BTW, what kind of improvements were observed in the test case under investigation? |
jason-simmons
force-pushed
the
bug_91382
branch
from
68651ff
to
1177490
Compare
|
It looked like there was a decrease in total memory usage similar to the size of the app snapshot. |
|
Sounds good. Are you planning on patching the test harness in this patch? |
jason-simmons
force-pushed
the
bug_91382
branch
from
1177490
to
a351ab7
Compare
|
Added support for the VM service snapshot in the test harness on Linux |
jason-simmons
force-pushed
the
bug_91382
branch
from
a351ab7
to
c36f067
Compare
…file mode on Android Fixes flutter/flutter#91382
jason-simmons
force-pushed
the
bug_91382
branch
from
c36f067
to
e384c10
Compare
|
Can this PR be merged ? |
jason-simmons
added
the
waiting for tree to go green
…e in profile mode on Android (flutter/engine#29245)
Fixes flutter/flutter#91382