Setup default font manager after engine created, to improve startup performance #18225

scutlight · 2020-05-08T09:29:06Z

Shell::Create may gain 25~50% performance improvement by this pr ^_^

SkFontMgr::RefDefault is time-consuming, about 15ms in Pixel2, which is blocking Android's main thread during engine initialization. It is called by FontCollection while creating Engine, and I think we can just make that happen right after initialization instead of initializing.

before this pr:

after this pr:

Shown as the above figures,
Shell::Create ~60ms ==> ~30ms
ShellSetupUISubsystem ~25ms ==> ~10ms

jason-simmons · 2020-05-08T14:53:39Z

This is creating a race between the task that sets up the font manager and users of the engine that assume that the engine is fully initialized after it is created.

Most of the engine initialization work is not done on the Android main thread. If the app is blocking the main thread during the call to FlutterLoader.ensureInitializationComplete, then consider using ensureInitializationCompleteAsync which will invoke a callback after engine setup finishes.

liyuqian · 2020-05-09T00:23:07Z

Cross reference #18047 and #17192. Those PRs also aim at speeding up startup time by making some work asynchronous. Most parts of shell and engine are initialized on other threads (e.g., UI thread), but it seems that the platform thread (Android's main thread) is currently blocked until all those initializations are finished (see, e.g., the future wait in

engine/shell/common/shell.cc

Line 167 in 9d8daf2

engine_future.get(), //

).

@jason-simmons : do we have a benchmark that verifies ensureInitializationCompleteAsync would allow Android main thread to be usable before the shell is fully initialized? If Shell::Create will block the platform thread before the Shell is fully initialized, I'm not sure whether ensureInitializationCompleteAsync could allow the app to do some platform thread work before that.

jason-simmons · 2020-05-09T01:22:51Z

ensureInitializationCompleteAsync will avoid blocking the main thread while waiting for one-time initialization work that is done by FlutterLoader in Java background threads.

But if the concern is the latency of Shell::CreateShellOnPlatformThread, then ensureInitializationCompleteAsync will not help. Reducing blocking there would involve making shell construction asynchronous (as in the linked PRs).

The default font manager provided by SkFontMgr::RefDefault() is a process-wide singleton owned by Skia. The first call to SkFontMgr::RefDefault() will take noticeable time, but later calls will return a reference to the preexisting font manager. If your app allows it, it might help to prefetch the default font manager by creating a FlutterEngine at a time when the main thread is less loaded.

If we can't make shell creation async, then another potential workaround would be calling SkFontMgr::RefDefault() during the part of FlutterLoader startup that runs on a background thread.

scutlight · 2020-05-09T01:48:29Z

Since SetupDefaultFontManager task is posted inside Shell::Setup, it should be the very next task run after engine created, and users of Shell can still assume that it is ready to use once shell has been created.

scutlight · 2020-05-09T02:02:03Z

If we can't make shell creation async, then another potential workaround would be calling SkFontMgr::RefDefault() during the part of FlutterLoader startup that runs on a background thread.

I've try this workaround by calling SkFontMgr::RefDefault() on a java background thread when libflutter.so loaded. As the first call to SkFontMgr::RefDefault() takes time, it blocks the engine construction a while.

scutlight · 2020-05-09T02:10:19Z

One more thing, I suppose that the first call to SkFontMgr::RefDefault() is expensive on all platforms not just Android. It's suggested a solution inside shell instead of platform embedding.

scutlight · 2020-05-18T03:26:36Z

@jason-simmons @liyuqian : Looking forward to your further comments.

scutlight · 2020-05-18T13:12:04Z

I've made a update.
As flutter_boost calls executeDartEntrypoint immediately after FlutterEngine was created, calling SkFontMgr::RefDefault() on a backgroud thread to pre-warm may be a seemly good workaround.
https://github.com/alibaba/flutter_boost/blob/bfb71ebd92b1c928d80881fe92a8fdff9a2552ad/android/src/main/java/com/idlefish/flutterboost/FlutterBoost.java#L158-L183

liyuqian · 2020-05-19T22:45:35Z

Just chatted with @jason-simmons offline. We think "there might be a race between the deferred task that calls SkFontMgr::RefDefault on the UI thread and any code that uses the engine to render text." Hence it's better to ensure that SkFontMgr::RefDefault is called before the engine is fully initialized.

Running SkFontMgr::RefDefault on another thread (not UI or platform thread) may be a way to parallelize the work and thus improve startup time as Jason suggested. Your screenshot in #18225 (comment) seems to suggest that SkFontMgr::RefDefault still runs on the UI thread, and thus it's not parallelized. Does your modification yesterday in 9788976 make it parallel and improve the startup time? In addition to the Java solution that's probably Android -specific, you may also try something like std::async(std::launch::async, [](){ SkFontMgr::RefDefault(); }); to make it cross-platform, and wait for its completion by the end of Shell::Create using std::promise, std::future.

BTW, how important is this improvement to your app, @scutlight? Is it "customer: critical" or "customer: blocker"? CC @zoeyfan for evaluating the importance from the customer's perspective.

jason-simmons

Discussed this with @chinmaygarde. The approach of deferring FontCollection::SetupDefaultFontManager to a task queued to the UI thread should be safe. The FontCollection is only accessed on the UI thread and the setup task will run before any other UI thread tasks that might use fonts.

One potential concern is that the background thread that calls SkFontMgr::RefDefault may not have completed when the UI thread task tries to access the RefDefault singleton. If that happens, then the UI thread will busy wait until the singleton is ready due to the implementation of Skia's SkOnce (see https://github.com/google/skia/blob/master/include/private/SkOnce.h#L44)

The PR does minimize the chance of that happening by starting the RefDefault immediately after libflutter.so is loaded.

jason-simmons · 2020-05-20T01:24:08Z

shell/common/engine.h

+  //----------------------------------------------------------------------------
+  /// @brief      Setup default font manager according to specific platform.
+  ///
+  /// @attention  This operation calls `SkFontMgr::RefDefault` which is


Remove the @attention comment. The call to SkFontMgr::RefDefault is an implementation detail that will vary across platforms.

jason-simmons · 2020-05-20T01:26:30Z

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

+   * singleton owned by Skia. Note that, the first call to SkFontMgr::RefDefault() will take
+   * noticeable time, but later calls will return a reference to the preexisting font manager.
+   */
+  public static native void nativeCreateDefaultFontManager();


Change this to nativePrefetchDefaultFontManager (and do the same for the CreateDefaultFontManager function in flutter_main.cc).

I'd prefer to avoid the name Create given that the function does not return a font manager instance.

jason-simmons · 2020-05-20T01:27:36Z

shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java

@@ -144,6 +144,17 @@ public InitResult call() {

            System.loadLibrary("flutter");

+            // Pre-warm the default font manager as soon as possible on a background thread.
+            // It helps to reduce time cost of engine setup that blocks the platform thread.
+            new Thread(


Use Executors.newSingleThreadExecutor().submit() for consistency with other threads used in FlutterLoader

jason-simmons · 2020-05-20T01:34:41Z

shell/platform/android/flutter_main.cc

@@ -155,6 +156,10 @@ void FlutterMain::SetupObservatoryUriCallback(JNIEnv* env) {
      });
 }

+static void CreateDefaultFontManager(JNIEnv* env, jclass jcaller) {
+  sk_sp<SkFontMgr> font_mgr(SkFontMgr::RefDefault());


Remove the sk_sp<SkFontMgr> and just call SkFontMgr::RefDefault(). Add a comment explaining that calling SkFontMgr::RefDefault will initialize a singleton owned by Skia.

scutlight · 2020-05-20T10:57:16Z

Just chatted with @jason-simmons offline. We think "there might be a race between the deferred task that calls SkFontMgr::RefDefault on the UI thread and any code that uses the engine to render text." Hence it's better to ensure that SkFontMgr::RefDefault is called before the engine is fully initialized.

It is clear that "the setup task will run before any other UI thread tasks that might use fonts", refer to Jason's latest #18225 (review), and my previous #18225 (comment)

Running SkFontMgr::RefDefault on another thread (not UI or platform thread) may be a way to parallelize the work and thus improve startup time as Jason suggested. Your screenshot in #18225 (comment) seems to suggest that SkFontMgr::RefDefault still runs on the UI thread, and thus it's not parallelized. Does your modification yesterday in 9788976 make it parallel and improve the startup time?

Sorry for confusing screenshot. It is parallelized. The UI thread is busy waiting until the singleton is ready due to the implementation of Skia's SkOnce called by SkFontMgr::RefDefault. That's also the reason why I move SetupDefaultFontManager out of engine creating.

In addition to the Java solution that's probably Android -specific, you may also try something like std::async(std::launch::async, [](){ SkFontMgr::RefDefault(); }); to make it cross-platform, and wait for its completion by the end of Shell::Create using std::promise, std::future.

Yeah, I agree that. Maybe we can put this std::async into PerformInitializationTasks of shell.cc to make it cross-platform. But I have no effort to cover tests to get the eventual result on other platforms. And more important, it's better to start SkFontMgr::RefDefault() as soon as possible because it really takes a long time to finish.

BTW, how important is this improvement to your app, @scutlight? Is it "customer: critical" or "customer: blocker"? CC @zoeyfan for evaluating the importance from the customer's perspective.

Maybe not so emergent as critical, but really important since the whole engine startup is non-trivial and blocking the Android's main thread.

liyuqian · 2020-05-20T21:15:41Z

Thanks @scutlight ! It seems that the data race is no longer a problem.

BTW I verified locally that our complex_layout__start_up test would catch 20-30ms speedup in its timeToFirstFrameRasterizationMicros metric:

	timeToFirstFrameMicros before	timeToFirstFrameMicros after
run 1	623960	587673
run 2	587092	516596
run 3	617480	579517
run 4	579387	600020
run 5	619112	587077
average	605406.2	574176.6

@xster @gaaclarke : the current PR only speeds up Android startup time. You might be interested in extending it to iOS as well.

…erformance.

gaaclarke · 2020-05-21T16:43:14Z

shell/common/shell.cc

+  fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(),
+                                    [engine = weak_engine_] {
+                                      if (engine) {
+                                        engine->SetupDefaultFontManager();


What protection do we have that someone won't use FontCollection.collection_ before this executes?

The FontCollection is only usable on the UI thread, and UI thread tasks are executed in the order that they were queued. Therefore the task that completes setup of the FontCollection will execute before any task that might query the FontCollection.

There could already be something in the UI event queue that will use the FontCollection though.

platform_thread->execute({ ui_thread->execute({ shell->setup(); }); ui_thread->execute({ someFontCollectionUsingFunction(); }); });

There is no guarantee in this case that SetupDefaultFontManager will execute before someFontCollectionUsingFunction.

edit: If this is how things work this should be safe. All usages of the font collection on the ui thread are given a reference to a shell that has already been setup. This might be the case.

platform_thread->execute({ shell->setup(); ui_thread->execute({ someFontCollectionUsingFunction(shell); }); });

The embedder APIs will not return a shell to the caller until Shell::Setup has completed and the SetupDefaultFontManager task has already been queued.

gaaclarke

I just want to make sure my question is answered before merge. It isn't obvious the protections we are doing here and I don't want to introduce a subtle race condition.

gaaclarke

Talked with @jason-simmons offline, while the thread-safety isn't asserted at compile-time he assures me that it is practically impossible to have something else in the UI event queue because of the way the embedder API works. I asked for a runtime assert if one was easy to insert. I'll leave that up to his judgement.

scutlight · 2020-05-26T02:13:57Z

@jason-simmons @gaaclarke : hi, since both of you have approved, shall this pr be merged?

jason-simmons · 2020-05-27T21:38:12Z

Sorry for the delay - the pipeline that rolls the engine into the Flutter framework has been having problems. I'm planning to merge this PR when the roller stabilizes.

…tartup performance (flutter/engine#18225)

liyuqian · 2020-05-29T19:07:30Z

Big regression detected: https://flutter-perf.skia.org/e/?begin=1590623523&end=1590741783&keys=X9f1bf2296ffa1906179257e3f4475f70&xbaroffset=17822 @scutlight @jason-simmons

liyuqian · 2020-05-29T19:14:06Z

BTW, there's also a big speedup https://flutter-perf.skia.org/e/?begin=1590623523&end=1590741783&keys=X0fffad7016a0440022353623dc52ae28&xbaroffset=17822

## Description As the related issue refer, the application may be doing too much work on its main thread even in a simple hello_world demo. That is because the creation of `Engine` on the ui thread takes a noticeable time, and it is blocking the platform thread in order to run `Shell::Setup` synchronously. The cost of `Engine`'s constructor is mainly about the creating of root isolate. Actually, there used to be another time-consuming process, the default font manager setup, which was resolved by #18225. Similar to #18225, this pr move the creation of root isolate out from creating `Engine`. After this action, the main thread blocking is quite an acceptable slice. ## Related Issues flutter/flutter#40563 could be resolved by this pr.

googlebot added cla: yes labels May 8, 2020

auto-assign bot requested a review from jason-simmons May 8, 2020 09:30

scutlight force-pushed the pr/setup-default-font-manager-after-engine-created branch 2 times, most recently from 0d297ba to 9788976 Compare May 19, 2020 02:17

jason-simmons requested changes May 20, 2020

View reviewed changes

scutlight force-pushed the pr/setup-default-font-manager-after-engine-created branch from 9788976 to 7f7358e Compare May 20, 2020 10:15

scutlight requested a review from jason-simmons May 20, 2020 10:17

jason-simmons approved these changes May 21, 2020

View reviewed changes

scutlight added 3 commits May 21, 2020 14:04

Setup default font manager after engine created, to improve startup p…

2999d0c

…erformance.

Pre-warm default font manager, to reduce time cost of engine setup

322cd0b

Update changes as review requested.

e5b6447

scutlight force-pushed the pr/setup-default-font-manager-after-engine-created branch from 7f7358e to e5b6447 Compare May 21, 2020 06:05

gaaclarke reviewed May 21, 2020

View reviewed changes

gaaclarke requested changes May 21, 2020

View reviewed changes

gaaclarke approved these changes May 21, 2020

View reviewed changes

liyuqian added perf: speed Performance issues related to (mostly rendering) speed severe: performance Relates to speed or footprint issues. labels May 26, 2020

jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 28, 2020

fluttergithubbot merged commit 89cf074 into flutter:master May 29, 2020

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 29, 2020

89cf074 Setup default font manager after engine created, to improve s…

5537208

…tartup performance (flutter/engine#18225)

engine-flutter-autoroll mentioned this pull request May 29, 2020

Roll Engine from c5d012900f7d to 17737e6fd4ec (6 revisions) flutter/flutter#58221

Merged

liyuqian mentioned this pull request May 29, 2020

BM_ShellShutdown regressed ~16x flutter/flutter#58261

Closed

scutlight mentioned this pull request May 30, 2020

Fix BM_ShellShutdown regression #18676

Merged

scutlight mentioned this pull request Jul 30, 2020

Create root isolate asynchronously #20142

Merged

12 tasks

scutlight mentioned this pull request Sep 1, 2020

Make Shell::Create not blocking platform thread by asynchronously setup shell subsystems #19641

Closed

12 tasks

jiahaog mentioned this pull request Jan 18, 2022

Make prefetchDefaultFontManager on Android happen in parallel with loading of the Dart isolate flutter/flutter#96767

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup default font manager after engine created, to improve startup performance #18225

Setup default font manager after engine created, to improve startup performance #18225

scutlight commented May 8, 2020

jason-simmons commented May 8, 2020

liyuqian commented May 9, 2020

jason-simmons commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 18, 2020

scutlight commented May 18, 2020

liyuqian commented May 19, 2020 •

edited

Loading

jason-simmons left a comment

jason-simmons May 20, 2020

scutlight May 20, 2020

jason-simmons May 20, 2020

scutlight May 20, 2020

jason-simmons May 20, 2020

scutlight May 20, 2020

jason-simmons May 20, 2020

scutlight May 20, 2020

scutlight commented May 20, 2020

liyuqian commented May 20, 2020 •

edited

Loading

gaaclarke May 21, 2020

jason-simmons May 21, 2020

gaaclarke May 21, 2020 •

edited

Loading

jason-simmons May 21, 2020

gaaclarke left a comment

gaaclarke left a comment

scutlight commented May 26, 2020

jason-simmons commented May 27, 2020

liyuqian commented May 29, 2020

liyuqian commented May 29, 2020

Setup default font manager after engine created, to improve startup performance #18225

Setup default font manager after engine created, to improve startup performance #18225

Conversation

scutlight commented May 8, 2020

jason-simmons commented May 8, 2020

liyuqian commented May 9, 2020

jason-simmons commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 9, 2020

scutlight commented May 18, 2020

scutlight commented May 18, 2020

liyuqian commented May 19, 2020 • edited Loading

jason-simmons left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scutlight commented May 20, 2020

liyuqian commented May 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaaclarke May 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaaclarke left a comment

Choose a reason for hiding this comment

gaaclarke left a comment

Choose a reason for hiding this comment

scutlight commented May 26, 2020

jason-simmons commented May 27, 2020

liyuqian commented May 29, 2020

liyuqian commented May 29, 2020

liyuqian commented May 19, 2020 •

edited

Loading

liyuqian commented May 20, 2020 •

edited

Loading

gaaclarke May 21, 2020 •

edited

Loading