-
Notifications
You must be signed in to change notification settings - Fork 6k
Move FlutterLoader disk I/O to a background thread to comply with Android strict mode #18241
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,9 @@ | |
| import io.flutter.view.VsyncWaiter; | ||
| import java.io.File; | ||
| import java.util.*; | ||
| import java.util.concurrent.Callable; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.Future; | ||
|
|
||
| /** Finds Flutter resources in an application APK and also loads Flutter's native library. */ | ||
| public class FlutterLoader { | ||
|
|
@@ -76,10 +79,23 @@ public static FlutterLoader getInstance() { | |
| } | ||
|
|
||
| private boolean initialized = false; | ||
| @Nullable private ResourceExtractor resourceExtractor; | ||
| @Nullable private Settings settings; | ||
| private long initStartTimestampMillis; | ||
|
|
||
| private static class InitResult { | ||
| final String appStoragePath; | ||
| final String engineCachesPath; | ||
| final String dataDirPath; | ||
|
|
||
| private InitResult(String appStoragePath, String engineCachesPath, String dataDirPath) { | ||
| this.appStoragePath = appStoragePath; | ||
| this.engineCachesPath = engineCachesPath; | ||
| this.dataDirPath = dataDirPath; | ||
| } | ||
| } | ||
|
|
||
| @Nullable Future<InitResult> initResultFuture; | ||
|
|
||
| /** | ||
| * Starts initialization of the native system. | ||
| * | ||
|
|
@@ -110,19 +126,35 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se | |
| } | ||
|
|
||
| // Ensure that the context is actually the application context. | ||
| applicationContext = applicationContext.getApplicationContext(); | ||
| final Context appContext = applicationContext.getApplicationContext(); | ||
|
|
||
| this.settings = settings; | ||
|
|
||
| initStartTimestampMillis = SystemClock.uptimeMillis(); | ||
| initConfig(applicationContext); | ||
| initResources(applicationContext); | ||
|
|
||
| System.loadLibrary("flutter"); | ||
|
|
||
| VsyncWaiter.getInstance( | ||
| (WindowManager) applicationContext.getSystemService(Context.WINDOW_SERVICE)) | ||
| initConfig(appContext); | ||
| VsyncWaiter.getInstance((WindowManager) appContext.getSystemService(Context.WINDOW_SERVICE)) | ||
| .init(); | ||
|
|
||
| // Use a background thread for initialization tasks that require disk access. | ||
| Callable<InitResult> initTask = | ||
| new Callable<InitResult>() { | ||
| @Override | ||
| public InitResult call() { | ||
| ResourceExtractor resourceExtractor = initResources(appContext); | ||
|
|
||
| System.loadLibrary("flutter"); | ||
|
|
||
| if (resourceExtractor != null) { | ||
| resourceExtractor.waitForCompletion(); | ||
| } | ||
|
|
||
| return new InitResult( | ||
| PathUtils.getFilesDir(appContext), | ||
| PathUtils.getCacheDirectory(appContext), | ||
| PathUtils.getDataDirectory(appContext)); | ||
| } | ||
| }; | ||
| initResultFuture = Executors.newSingleThreadExecutor().submit(initTask); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -147,9 +179,7 @@ public void ensureInitializationComplete( | |
| "ensureInitializationComplete must be called after startInitialization"); | ||
| } | ||
| try { | ||
| if (resourceExtractor != null) { | ||
| resourceExtractor.waitForCompletion(); | ||
| } | ||
| InitResult result = initResultFuture.get(); | ||
|
|
||
| List<String> shellArgs = new ArrayList<>(); | ||
| shellArgs.add("--icu-symbol-prefix=_binary_icudtl_dat"); | ||
|
|
@@ -167,8 +197,7 @@ public void ensureInitializationComplete( | |
|
|
||
| String kernelPath = null; | ||
| if (BuildConfig.DEBUG || BuildConfig.JIT_RELEASE) { | ||
| String snapshotAssetPath = | ||
| PathUtils.getDataDirectory(applicationContext) + File.separator + flutterAssetsDir; | ||
| String snapshotAssetPath = result.dataDirPath + File.separator + flutterAssetsDir; | ||
| kernelPath = snapshotAssetPath + File.separator + DEFAULT_KERNEL_BLOB; | ||
| shellArgs.add("--" + SNAPSHOT_ASSET_PATH_KEY + "=" + snapshotAssetPath); | ||
| shellArgs.add("--" + VM_SNAPSHOT_DATA_KEY + "=" + vmSnapshotData); | ||
|
|
@@ -188,20 +217,18 @@ public void ensureInitializationComplete( | |
| + aotSharedLibraryName); | ||
| } | ||
|
|
||
| shellArgs.add("--cache-dir-path=" + PathUtils.getCacheDirectory(applicationContext)); | ||
| shellArgs.add("--cache-dir-path=" + result.engineCachesPath); | ||
| if (settings.getLogTag() != null) { | ||
| shellArgs.add("--log-tag=" + settings.getLogTag()); | ||
| } | ||
|
|
||
| String appStoragePath = PathUtils.getFilesDir(applicationContext); | ||
| String engineCachesPath = PathUtils.getCacheDirectory(applicationContext); | ||
| long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; | ||
| FlutterJNI.nativeInit( | ||
| applicationContext, | ||
| shellArgs.toArray(new String[0]), | ||
| kernelPath, | ||
| appStoragePath, | ||
| engineCachesPath, | ||
| result.appStoragePath, | ||
| result.engineCachesPath, | ||
| initTimeMillis); | ||
|
|
||
| initialized = true; | ||
|
|
@@ -232,12 +259,17 @@ public void ensureInitializationCompleteAsync( | |
| callbackHandler.post(callback); | ||
| return; | ||
| } | ||
| new Thread( | ||
| Executors.newSingleThreadExecutor() | ||
| .execute( | ||
| new Runnable() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we use an executor here too for consistency?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| @Override | ||
| public void run() { | ||
| if (resourceExtractor != null) { | ||
| resourceExtractor.waitForCompletion(); | ||
| InitResult result; | ||
| try { | ||
| result = initResultFuture.get(); | ||
| } catch (Exception e) { | ||
| Log.e(TAG, "Flutter initialization failed.", e); | ||
| throw new RuntimeException(e); | ||
| } | ||
| new Handler(Looper.getMainLooper()) | ||
| .post( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a comment for line 288. Is FlutterJNI.nativeInit the only line from ensureInitializationComplete that really needs to be run on the main thread? Could we potentially move everything into the init task to maximize the things not running on the main thread?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rest of |
||
|
|
@@ -250,8 +282,7 @@ public void run() { | |
| } | ||
| }); | ||
| } | ||
| }) | ||
| .start(); | ||
| }); | ||
| } | ||
|
|
||
| @NonNull | ||
|
|
@@ -289,7 +320,8 @@ private void initConfig(@NonNull Context applicationContext) { | |
| } | ||
|
|
||
| /** Extract assets out of the APK that need to be cached as uncompressed files on disk. */ | ||
| private void initResources(@NonNull Context applicationContext) { | ||
| private ResourceExtractor initResources(@NonNull Context applicationContext) { | ||
| ResourceExtractor resourceExtractor = null; | ||
| if (BuildConfig.DEBUG || BuildConfig.JIT_RELEASE) { | ||
| final String dataDirPath = PathUtils.getDataDirectory(applicationContext); | ||
| final String packageName = applicationContext.getPackageName(); | ||
|
|
@@ -307,6 +339,7 @@ private void initResources(@NonNull Context applicationContext) { | |
|
|
||
| resourceExtractor.start(); | ||
| } | ||
| return resourceExtractor; | ||
| } | ||
|
|
||
| @NonNull | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // Copyright 2018 The Chromium Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| package dev.flutter.scenarios; | ||
|
|
||
| import android.os.StrictMode; | ||
| import io.flutter.app.FlutterApplication; | ||
|
|
||
| public class ScenarioApplication extends FlutterApplication { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just double checking. Our CI doesn't run this yet so I'm assuming you ran it manually to check.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes - I've run this manually on Firebase and on local devices. The LUCI script is attempting to run the scenario app test on Firebase (https://flutter.googlesource.com/recipes/+/refs/heads/master/recipes/engine.py#560). But it looks like this is currently broken on the LUCI bots. |
||
| @Override | ||
| public void onCreate() { | ||
| StrictMode.setThreadPolicy( | ||
| new StrictMode.ThreadPolicy.Builder() | ||
| .detectDiskReads() | ||
| .detectDiskWrites() | ||
| .penaltyDeath() | ||
| .build()); | ||
| super.onCreate(); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love that we finally stopped using the resource extractor as a unit of transaction now since it was really confusing. Though this is close to getting us all the way. i.e. upon initialization task complete, initialization isn't quite done yet. We still need to wait for the resource extractor to finish. Perhaps just chain the resource extractor completion inside this async task?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done