-
Notifications
You must be signed in to change notification settings - Fork 6k
Limit heap growth on Android #20473
Limit heap growth on Android #20473
Changes from 2 commits
cd9045e
de357b2
834417b
1ed2d8d
5bebd55
eb485ef
0847dcd
f2a9f84
96ad46e
0a345dd
4612bac
a118c90
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 |
|---|---|---|
|
|
@@ -60,22 +60,26 @@ public static FlutterLoader getInstance() { | |
| return instance; | ||
| } | ||
|
|
||
| /** Creates a {@code FlutterLoader} that uses a default constructed {@link FlutterJNI}. */ | ||
| public FlutterLoader() { | ||
| this(null); | ||
| this(new FlutterJNI()); | ||
|
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 nicer |
||
| } | ||
|
|
||
| public FlutterLoader(@Nullable FlutterJNI flutterJNILoader) { | ||
| if (flutterJNILoader == null) { | ||
| flutterJNILoader = new FlutterJNI(); | ||
| } | ||
| this.flutterJNILoader = flutterJNILoader; | ||
| /** | ||
| * Creates a {@code FlutterLoader} with the specified {@link FlutterJNI}. | ||
| * | ||
| * @param flutterJNI The {@link FlutterJNI} instance to use for loading the libflutter.so C++ | ||
| * library, setting up the font manager, and calling into C++ initalization. | ||
| */ | ||
| public FlutterLoader(@NonNull FlutterJNI flutterJNI) { | ||
| this.flutterJNI = flutterJNI; | ||
| } | ||
|
|
||
| private boolean initialized = false; | ||
| @Nullable private Settings settings; | ||
| private long initStartTimestampMillis; | ||
| private FlutterApplicationInfo flutterApplicationInfo; | ||
| private FlutterJNI flutterJNILoader; | ||
| private FlutterJNI flutterJNI; | ||
|
|
||
| private static class InitResult { | ||
| final String appStoragePath; | ||
|
|
@@ -137,7 +141,7 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se | |
| public InitResult call() { | ||
| ResourceExtractor resourceExtractor = initResources(appContext); | ||
|
|
||
| flutterJNILoader.loadLibrary(); | ||
| flutterJNI.loadLibrary(); | ||
|
|
||
| // Prefetch 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. | ||
|
|
@@ -146,7 +150,7 @@ public InitResult call() { | |
| new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| flutterJNILoader.prefetchDefaultFontManager(); | ||
| flutterJNI.prefetchDefaultFontManager(); | ||
| } | ||
| }); | ||
|
|
||
|
|
@@ -242,7 +246,7 @@ public void ensureInitializationComplete( | |
|
|
||
| long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; | ||
|
|
||
| flutterJNILoader.init( | ||
| flutterJNI.init( | ||
| applicationContext, | ||
| shellArgs.toArray(new String[0]), | ||
| kernelPath, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,15 +32,13 @@ public void setUp() { | |
| @Test | ||
| public void pluginsCanAccessFlutterAssetPaths() { | ||
| // Setup test. | ||
| FlutterJNI mockFlutterJNILoader = mock(FlutterJNI.class); | ||
| FlutterJNI mockFlutterJNI = mock(FlutterJNI.class); | ||
| FlutterInjector.setInstance( | ||
|
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. Cool, lg. Can you push some changes to flutter/website#4561 directly for the new pattern?
Contributor
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. |
||
| new FlutterInjector.Builder() | ||
| .setFlutterLoader(new FlutterLoader(mockFlutterJNILoader)) | ||
| .build()); | ||
| new FlutterInjector.Builder().setFlutterLoader(new FlutterLoader(mockFlutterJNI)).build()); | ||
| FlutterJNI flutterJNI = mock(FlutterJNI.class); | ||
| when(flutterJNI.isAttached()).thenReturn(true); | ||
|
|
||
| FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNILoader); | ||
| FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI); | ||
|
|
||
| // Execute behavior under test. | ||
| FlutterEngine flutterEngine = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,8 @@ | |
| import android.app.ActivityManager; | ||
| import android.content.Context; | ||
| import io.flutter.embedding.engine.FlutterJNI; | ||
| import java.util.*; | ||
| import java.util.Arrays; | ||
| import java.util.List; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.mockito.ArgumentCaptor; | ||
|
|
@@ -38,21 +39,32 @@ public void itReportsUninitializedAfterCreating() { | |
|
|
||
| @Test | ||
| public void itReportsInitializedAfterInitializing() { | ||
| FlutterJNI mockFlutterJNILoader = mock(FlutterJNI.class); | ||
| FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNILoader); | ||
| FlutterJNI mockFlutterJNI = mock(FlutterJNI.class); | ||
| FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI); | ||
|
|
||
| assertFalse(flutterLoader.initialized()); | ||
| flutterLoader.startInitialization(RuntimeEnvironment.application); | ||
| flutterLoader.ensureInitializationComplete(RuntimeEnvironment.application, null); | ||
| shadowOf(getMainLooper()).idle(); | ||
| assertTrue(flutterLoader.initialized()); | ||
| verify(mockFlutterJNILoader, times(1)).loadLibrary(); | ||
| verify(mockFlutterJNI, times(1)).loadLibrary(); | ||
| } | ||
|
|
||
| @Test | ||
| public void itSetsTheOldGenHeapSizeAppropriately() { | ||
| FlutterJNI mockFlutterJNI = mock(FlutterJNI.class); | ||
| FlutterLoader flutterLoader = new FlutterLoader(mockFlutterJNI); | ||
|
|
||
| assertFalse(flutterLoader.initialized()); | ||
| flutterLoader.startInitialization(RuntimeEnvironment.application); | ||
| flutterLoader.ensureInitializationComplete(RuntimeEnvironment.application, null); | ||
| shadowOf(getMainLooper()).idle(); | ||
|
|
||
| ActivityManager activityManager = | ||
|
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. can this be a separate test? Since it seems unrelated to the test above.
Contributor
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 |
||
| (ActivityManager) RuntimeEnvironment.application.getSystemService(Context.ACTIVITY_SERVICE); | ||
| final String oldGenHeapArg = "--old-gen-heap-size=" + activityManager.getLargeMemoryClass(); | ||
| ArgumentCaptor<String[]> shellArgsCaptor = ArgumentCaptor.forClass(String[].class); | ||
| verify(mockFlutterJNILoader, times(1)) | ||
| verify(mockFlutterJNI, times(1)) | ||
| .init( | ||
| eq(RuntimeEnvironment.application), | ||
| shellArgsCaptor.capture(), | ||
|
|
||
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.
Docs
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.