From 24ff75fb08190bdfa59678091cb9d02a2945f373 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 22 Feb 2017 19:05:06 -0800 Subject: [PATCH 1/6] Update package use info when the app data is updated - clear usesByOtherApps flag when the package is updated - delete secondary dex usage data when the app data is destroyed Test: runtest -x .../PackageDexUsageTests.java runtest -x .../DexManagerTests.java Bug: 32871170 Bug: 35381405 (cherry picked from commit 99dd37b3c5262910150ef955d16a33d32da264dd) Merged-In: I3a249b9e8680e745fa678c7ce61b4ae764078fb9 Change-Id: Ia8416e7232cda3e42a8dccd51cb152a237e0f317 --- .../server/pm/PackageManagerService.java | 6 + .../com/android/server/pm/dex/DexManager.java | 96 +++++++++++--- .../server/pm/dex/PackageDexUsage.java | 33 +++++ .../server/pm/dex/DexManagerTests.java | 118 +++++++++++++++++- .../server/pm/dex/PackageDexUsageTests.java | 50 ++++++++ 5 files changed, 283 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 42f650247c209..5945ed04e321e 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -7738,6 +7738,7 @@ public class PackageManagerService extends IPackageManager.Stub { } catch (InstallerException e) { Slog.w(TAG, String.valueOf(e)); } + mDexManager.notifyPackageDataDestroyed(pkg.packageName, userId); } } @@ -14449,6 +14450,8 @@ public class PackageManagerService extends IPackageManager.Stub { } prepareAppDataAfterInstallLIF(newPackage); addedPkg = true; + mDexManager.notifyPackageUpdated(newPackage.packageName, + newPackage.baseCodePath, newPackage.splitCodePaths); } catch (PackageManagerException e) { res.setError("Package couldn't be installed in " + pkg.codePath, e); } @@ -14596,6 +14599,9 @@ public class PackageManagerService extends IPackageManager.Stub { updateSettingsLI(newPackage, installerPackageName, allUsers, res, user); prepareAppDataAfterInstallLIF(newPackage); + + mDexManager.notifyPackageUpdated(newPackage.packageName, + newPackage.baseCodePath, newPackage.splitCodePaths); } } catch (PackageManagerException e) { res.setReturnCode(INSTALL_FAILED_INTERNAL_ERROR); diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 01124e2ee8358..755c486d1be5d 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -22,6 +22,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageParser; import android.os.RemoteException; import android.os.storage.StorageManager; +import android.os.UserHandle; import android.util.Slog; @@ -179,17 +180,64 @@ public class DexManager { } } - public void notifyPackageInstalled(PackageInfo info, int userId) { - cachePackageCodeLocation(info, userId); + /** + * Notifies that a new package was installed for {@code userId}. + * {@code userId} must not be {@code UserHandle.USER_ALL}. + * + * @throws IllegalArgumentException if {@code userId} is {@code UserHandle.USER_ALL}. + */ + public void notifyPackageInstalled(PackageInfo pi, int userId) { + if (userId == UserHandle.USER_ALL) { + throw new IllegalArgumentException( + "notifyPackageInstalled called with USER_ALL"); + } + cachePackageCodeLocation(pi.packageName, pi.applicationInfo.sourceDir, + pi.applicationInfo.splitSourceDirs, pi.applicationInfo.dataDir, userId); } - private void cachePackageCodeLocation(PackageInfo info, int userId) { - PackageCodeLocations pcl = mPackageCodeLocationsCache.get(info.packageName); - if (pcl != null) { - pcl.mergeAppDataDirs(info.applicationInfo, userId); - } else { - mPackageCodeLocationsCache.put(info.packageName, - new PackageCodeLocations(info.applicationInfo, userId)); + /** + * Notifies that package {@code packageName} was updated. + * This will clear the UsedByOtherApps mark if it exists. + */ + public void notifyPackageUpdated(String packageName, String baseCodePath, + String[] splitCodePaths) { + cachePackageCodeLocation(packageName, baseCodePath, splitCodePaths, null, /*userId*/ -1); + // In case there was an update, write the package use info to disk async. + // Note that we do the writing here and not in PackageDexUsage in order to be + // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs + // multiple updates in PackaeDexUsage before writing it). + if (mPackageDexUsage.clearUsedByOtherApps(packageName)) { + mPackageDexUsage.maybeWriteAsync(); + } + } + + /** + * Notifies that the user {@code userId} data for package {@code packageName} + * was destroyed. This will remove all usage info associated with the package + * for the given user. + * {@code userId} is allowed to be {@code UserHandle.USER_ALL} in which case + * all usage information for the package will be removed. + */ + public void notifyPackageDataDestroyed(String packageName, int userId) { + boolean updated = userId == UserHandle.USER_ALL + ? mPackageDexUsage.removePackage(packageName) + : mPackageDexUsage.removeUserPackage(packageName, userId); + // In case there was an update, write the package use info to disk async. + // Note that we do the writing here and not in PackageDexUsage in order to be + // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs + // multiple updates in PackaeDexUsage before writing it). + if (updated) { + mPackageDexUsage.maybeWriteAsync(); + } + } + + public void cachePackageCodeLocation(String packageName, String baseCodePath, + String[] splitCodePaths, String dataDir, int userId) { + PackageCodeLocations pcl = putIfAbsent(mPackageCodeLocationsCache, packageName, + new PackageCodeLocations(packageName, baseCodePath, splitCodePaths)); + pcl.updateCodeLocation(baseCodePath, splitCodePaths); + if (dataDir != null) { + pcl.mergeAppDataDirs(dataDir, userId); } } @@ -202,7 +250,8 @@ public class DexManager { int userId = entry.getKey(); for (PackageInfo pi : packageInfoList) { // Cache the code locations. - cachePackageCodeLocation(pi, userId); + cachePackageCodeLocation(pi.packageName, pi.applicationInfo.sourceDir, + pi.applicationInfo.splitSourceDirs, pi.applicationInfo.dataDir, userId); // Cache a map from package name to the set of user ids who installed the package. // We will use it to sync the data and remove obsolete entries from @@ -425,27 +474,36 @@ public class DexManager { */ private static class PackageCodeLocations { private final String mPackageName; - private final String mBaseCodePath; + private String mBaseCodePath; private final Set mSplitCodePaths; // Maps user id to the application private directory. private final Map> mAppDataDirs; public PackageCodeLocations(ApplicationInfo ai, int userId) { - mPackageName = ai.packageName; - mBaseCodePath = ai.sourceDir; + this(ai.packageName, ai.sourceDir, ai.splitSourceDirs); + mergeAppDataDirs(ai.dataDir, userId); + } + public PackageCodeLocations(String packageName, String baseCodePath, + String[] splitCodePaths) { + mPackageName = packageName; mSplitCodePaths = new HashSet<>(); - if (ai.splitSourceDirs != null) { - for (String split : ai.splitSourceDirs) { + mAppDataDirs = new HashMap<>(); + updateCodeLocation(baseCodePath, splitCodePaths); + } + + public void updateCodeLocation(String baseCodePath, String[] splitCodePaths) { + mBaseCodePath = baseCodePath; + mSplitCodePaths.clear(); + if (splitCodePaths != null) { + for (String split : splitCodePaths) { mSplitCodePaths.add(split); } } - mAppDataDirs = new HashMap<>(); - mergeAppDataDirs(ai, userId); } - public void mergeAppDataDirs(ApplicationInfo ai, int userId) { + public void mergeAppDataDirs(String dataDir, int userId) { Set dataDirs = putIfAbsent(mAppDataDirs, userId, new HashSet<>()); - dataDirs.add(ai.dataDir); + dataDirs.add(dataDir); } public int searchDex(String dexPath, int userId) { diff --git a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java index 3693bce04eb10..8a66f12cb6d9d 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -376,8 +376,35 @@ public class PackageDexUsage extends AbstractStatsBase { } } + /** + * Clears the {@code usesByOtherApps} marker for the package {@code packageName}. + * @return true if the package usage info was updated. + */ + public boolean clearUsedByOtherApps(String packageName) { + synchronized (mPackageUseInfoMap) { + PackageUseInfo packageUseInfo = mPackageUseInfoMap.get(packageName); + if (packageUseInfo == null || !packageUseInfo.mIsUsedByOtherApps) { + return false; + } + packageUseInfo.mIsUsedByOtherApps = false; + return true; + } + } + + /** + * Remove the usage data associated with package {@code packageName}. + * @return true if the package usage was found and removed successfully. + */ + public boolean removePackage(String packageName) { + synchronized (mPackageUseInfoMap) { + return mPackageUseInfoMap.remove(packageName) != null; + } + } + /** * Remove all the records about package {@code packageName} belonging to user {@code userId}. + * If the package is left with no records of secondary dex usage and is not used by other + * apps it will be removed as well. * @return true if the record was found and actually deleted, * false if the record doesn't exist */ @@ -397,6 +424,12 @@ public class PackageDexUsage extends AbstractStatsBase { updated = true; } } + // If no secondary dex info is left and the package is not used by other apps + // remove the data since it is now useless. + if (packageUseInfo.mDexUseInfoMap.isEmpty() && !packageUseInfo.mIsUsedByOtherApps) { + mPackageUseInfoMap.remove(packageName); + updated = true; + } return updated; } } diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java index 90a2ec0c76289..fa0bd392f75de 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java @@ -16,9 +16,10 @@ package com.android.server.pm.dex; -import android.os.Build; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; +import android.os.Build; +import android.os.UserHandle; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; @@ -57,6 +58,7 @@ public class DexManagerTests { private int mUser0; private int mUser1; + @Before public void setup() { @@ -243,6 +245,113 @@ public class DexManagerTests { assertSecondaryUse(newPackage, pui, newSecondaries, /*isUsedByOtherApps*/false, mUser0); } + @Test + public void testNotifyPackageUpdated() { + // Foo loads Bar main apks. + notifyDexLoad(mFooUser0, mBarUser0.getBaseAndSplitDexPaths(), mUser0); + + // Bar is used by others now and should be in our records. + PackageUseInfo pui = getPackageUseInfo(mBarUser0); + assertNotNull(pui); + assertTrue(pui.isUsedByOtherApps()); + assertTrue(pui.getDexUseInfoMap().isEmpty()); + + // Notify that bar is updated. + mDexManager.notifyPackageUpdated(mBarUser0.getPackageName(), + mBarUser0.mPackageInfo.applicationInfo.sourceDir, + mBarUser0.mPackageInfo.applicationInfo.splitSourceDirs); + + // The usedByOtherApps flag should be clear now. + pui = getPackageUseInfo(mBarUser0); + assertNotNull(pui); + assertFalse(pui.isUsedByOtherApps()); + } + + @Test + public void testNotifyPackageUpdatedCodeLocations() { + // Simulate a split update. + String newSplit = mBarUser0.replaceLastSplit(); + List newSplits = new ArrayList<>(); + newSplits.add(newSplit); + + // We shouldn't find yet the new split as we didn't notify the package update. + notifyDexLoad(mFooUser0, newSplits, mUser0); + PackageUseInfo pui = getPackageUseInfo(mBarUser0); + assertNull(pui); + + // Notify that bar is updated. splitSourceDirs will contain the updated path. + mDexManager.notifyPackageUpdated(mBarUser0.getPackageName(), + mBarUser0.mPackageInfo.applicationInfo.sourceDir, + mBarUser0.mPackageInfo.applicationInfo.splitSourceDirs); + + // Now, when the split is loaded we will find it and we should mark Bar as usedByOthers. + notifyDexLoad(mFooUser0, newSplits, mUser0); + pui = getPackageUseInfo(mBarUser0); + assertNotNull(pui); + assertTrue(pui.isUsedByOtherApps()); + } + + @Test + public void testNotifyPackageDataDestroyForOne() { + // Bar loads its own secondary files. + notifyDexLoad(mBarUser0, mBarUser0.getSecondaryDexPaths(), mUser0); + notifyDexLoad(mBarUser1, mBarUser1.getSecondaryDexPaths(), mUser1); + + mDexManager.notifyPackageDataDestroyed(mBarUser0.getPackageName(), mUser0); + + // Bar should not be around since it was removed for all users. + PackageUseInfo pui = getPackageUseInfo(mBarUser1); + assertNotNull(pui); + assertSecondaryUse(mBarUser1, pui, mBarUser1.getSecondaryDexPaths(), + /*isUsedByOtherApps*/false, mUser1); + } + + @Test + public void testNotifyPackageDataDestroyForeignUse() { + // Foo loads its own secondary files. + List fooSecondaries = mFooUser0.getSecondaryDexPaths(); + notifyDexLoad(mFooUser0, fooSecondaries, mUser0); + + // Bar loads Foo main apks. + notifyDexLoad(mBarUser0, mFooUser0.getBaseAndSplitDexPaths(), mUser0); + + mDexManager.notifyPackageDataDestroyed(mFooUser0.getPackageName(), mUser0); + + // Foo should still be around since it's used by other apps but with no + // secondary dex info. + PackageUseInfo pui = getPackageUseInfo(mFooUser0); + assertNotNull(pui); + assertTrue(pui.isUsedByOtherApps()); + assertTrue(pui.getDexUseInfoMap().isEmpty()); + } + + @Test + public void testNotifyPackageDataDestroyComplete() { + // Foo loads its own secondary files. + List fooSecondaries = mFooUser0.getSecondaryDexPaths(); + notifyDexLoad(mFooUser0, fooSecondaries, mUser0); + + mDexManager.notifyPackageDataDestroyed(mFooUser0.getPackageName(), mUser0); + + // Foo should not be around since all its secondary dex info were deleted + // and it is not used by other apps. + PackageUseInfo pui = getPackageUseInfo(mFooUser0); + assertNull(pui); + } + + @Test + public void testNotifyPackageDataDestroyForAll() { + // Foo loads its own secondary files. + notifyDexLoad(mBarUser0, mBarUser0.getSecondaryDexPaths(), mUser0); + notifyDexLoad(mBarUser1, mBarUser1.getSecondaryDexPaths(), mUser1); + + mDexManager.notifyPackageDataDestroyed(mBarUser0.getPackageName(), UserHandle.USER_ALL); + + // Bar should not be around since it was removed for all users. + PackageUseInfo pui = getPackageUseInfo(mBarUser0); + assertNull(pui); + } + private void assertSecondaryUse(TestData testData, PackageUseInfo pui, List secondaries, boolean isUsedByOtherApps, int ownerUserId) { for (String dex : secondaries) { @@ -317,5 +426,12 @@ public class DexManagerTests { } return paths; } + + String replaceLastSplit() { + int length = mPackageInfo.applicationInfo.splitSourceDirs.length; + // Add an extra bogus dex extension to simulate a new split name. + mPackageInfo.applicationInfo.splitSourceDirs[length - 1] += ".dex"; + return mPackageInfo.applicationInfo.splitSourceDirs[length - 1]; + } } } diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java index 19e0bcf6ba56d..2e99433149eae 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDexUsageTests.java @@ -256,6 +256,30 @@ public class PackageDexUsageTests { assertNull(mPackageDexUsage.getPackageUseInfo(mFooBaseUser0.mPackageName)); } + @Test + public void testRemovePackage() { + // Record Bar secondaries for two different users. + assertTrue(record(mBarSecondary1User0)); + assertTrue(record(mBarSecondary2User1)); + + // Remove the package. + assertTrue(mPackageDexUsage.removePackage(mBarSecondary1User0.mPackageName)); + // Assert that we can't find the package anymore. + assertNull(mPackageDexUsage.getPackageUseInfo(mBarSecondary1User0.mPackageName)); + } + + @Test + public void testRemoveNonexistentPackage() { + // Record Bar secondaries for two different users. + assertTrue(record(mBarSecondary1User0)); + + // Remove the package. + assertTrue(mPackageDexUsage.removePackage(mBarSecondary1User0.mPackageName)); + // Remove the package again. It should return false because the package no longer + // has a record in the use info. + assertFalse(mPackageDexUsage.removePackage(mBarSecondary1User0.mPackageName)); + } + @Test public void testRemoveUserPackage() { // Record Bar secondaries for two different users. @@ -282,6 +306,32 @@ public class PackageDexUsageTests { assertPackageDexUsage(null, mBarSecondary2User1); } + @Test + public void testClearUsedByOtherApps() { + // Write a package which is used by other apps. + assertTrue(record(mFooSplit2UsedByOtherApps0)); + assertTrue(mPackageDexUsage.clearUsedByOtherApps(mFooSplit2UsedByOtherApps0.mPackageName)); + + // Check that the package is no longer used by other apps. + TestData noLongerUsedByOtherApps = new TestData( + mFooSplit2UsedByOtherApps0.mPackageName, + mFooSplit2UsedByOtherApps0.mDexFile, + mFooSplit2UsedByOtherApps0.mOwnerUserId, + mFooSplit2UsedByOtherApps0.mLoaderIsa, + /*mIsUsedByOtherApps*/false, + mFooSplit2UsedByOtherApps0.mPrimaryOrSplit); + assertPackageDexUsage(noLongerUsedByOtherApps); + } + + @Test + public void testClearUsedByOtherAppsNonexistent() { + // Write a package which is used by other apps. + assertTrue(record(mFooSplit2UsedByOtherApps0)); + assertTrue(mPackageDexUsage.clearUsedByOtherApps(mFooSplit2UsedByOtherApps0.mPackageName)); + // Clearing again should return false as there should be no update on the use info. + assertFalse(mPackageDexUsage.clearUsedByOtherApps(mFooSplit2UsedByOtherApps0.mPackageName)); + } + private void assertPackageDexUsage(TestData primary, TestData... secondaries) { String packageName = primary == null ? secondaries[0].mPackageName : primary.mPackageName; boolean primaryUsedByOtherApps = primary == null ? false : primary.mUsedByOtherApps; From b3e74b2f90e18d3246bbe15ae7b4496e77266b84 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Fri, 10 Mar 2017 18:24:33 -0800 Subject: [PATCH 2/6] Add missing return in DexManager Harmless issue but which can spam the logs. Test: mostly manual since the check is buried deep inside and cannot be verified accurately. I added another test to DexManager to stress that code path and then checked the logs. Bug: 36117123 (cherry picked from commit 2dfc1b3e125860221bc67835c2d5c99198a12f8a) Merged-In: I1a878a200f3f726dfaa85f1bed1398acc8dce979 Change-Id: Ib2c034add2e8229b90eb4aaa0067d42d26798670 --- .../java/com/android/server/pm/dex/DexManager.java | 2 +- .../src/com/android/server/pm/dex/DexManagerTests.java | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 755c486d1be5d..83dd392988bfc 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -432,7 +432,7 @@ public class DexManager { // Ignore framework code. // TODO(calin): is there a better way to detect it? if (dexPath.startsWith("/system/framework/")) { - new DexSearchResult("framework", DEX_SEARCH_NOT_FOUND); + return new DexSearchResult("framework", DEX_SEARCH_NOT_FOUND); } // First, check if the package which loads the dex file actually owns it. diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java index fa0bd392f75de..72fb78e89ea21 100644 --- a/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java @@ -61,7 +61,6 @@ public class DexManagerTests { @Before public void setup() { - mUser0 = 0; mUser1 = 1; @@ -352,6 +351,15 @@ public class DexManagerTests { assertNull(pui); } + @Test + public void testNotifyFrameworkLoad() { + String frameworkDex = "/system/framework/com.android.location.provider.jar"; + // Load a dex file from framework. + notifyDexLoad(mFooUser0, Arrays.asList(frameworkDex), mUser0); + // The dex file should not be recognized as a package. + assertNull(mDexManager.getPackageUseInfo(frameworkDex)); + } + private void assertSecondaryUse(TestData testData, PackageUseInfo pui, List secondaries, boolean isUsedByOtherApps, int ownerUserId) { for (String dex : secondaries) { From 37fb57084b78d32e3930d3aa07840d8762a0fff0 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Wed, 15 Mar 2017 18:38:57 -0700 Subject: [PATCH 3/6] Set pm.BackgroundDexOptService as the source of true The real dexopt maintainance job is com.android.server.pm.BackgroundDexOptService, and not com.android.server.BackgroundDexOptJobService Partial revert of commit 096d304ae3d85c1bfcda1a1d9cd4eb13d0815500. Test: manual inspection Bug: 36140426 (partial cherry picked from commit 91d40f1baa2eeed2aa68d2492e20650d5e71dab5) Merged-In: I983ac91117f107282095fa7eefdbce08e0dcfce3 Change-Id: I983ac91117f107282095fa7eefdbce08e0dcfce3 # Conflicts: # core/res/AndroidManifest.xml # services/core/java/com/android/server/pm/PackageManagerService.java # services/java/com/android/server/SystemServer.java --- .../java/com/android/server/pm/BackgroundDexOptService.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/BackgroundDexOptService.java b/services/core/java/com/android/server/pm/BackgroundDexOptService.java index 7aa96cfddc626..d8900c0546fd6 100644 --- a/services/core/java/com/android/server/pm/BackgroundDexOptService.java +++ b/services/core/java/com/android/server/pm/BackgroundDexOptService.java @@ -49,8 +49,6 @@ public class BackgroundDexOptService extends JobService { private static final boolean DEBUG = false; - private static final long RETRY_LATENCY = 4 * AlarmManager.INTERVAL_HOUR; - private static final int JOB_IDLE_OPTIMIZE = 800; private static final int JOB_POST_BOOT_UPDATE = 801; From d9732c8dbb23f51101d044c81b54d84d7e5492c7 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Mon, 13 Mar 2017 19:02:32 -0700 Subject: [PATCH 4/6] Move DexLoadReporter out of LoadedApk The DexLoadReporter was part of LoadedApk in order to lazily initialize it when the first class loader of the app was created. However there's no real association between the two and doing the initialization in LoadedApk buys us nothing. Extract the reporter in its own class and set it to BaseDexClassLoader during bindApplication stage. Test: boot, and check that loaded dex files are reported Bug: 32871170 Bug: 26719109 (cherry picked from commit 37dfc8ee3ec676a0f059a9f9f1ec5d273e78a502) Merged-In: I9a0e734ae4c16d5d0979aa6d0061fbc9b6e144f6 Change-Id: I9a0e734ae4c16d5d0979aa6d0061fbc9b6e144f6 # Conflicts: # core/java/android/app/LoadedApk.java --- core/java/android/app/ActivityThread.java | 11 +++++ core/java/android/app/DexLoadReporter.java | 57 ++++++++++++++++++++++ core/java/android/app/LoadedApk.java | 35 ------------- 3 files changed, 68 insertions(+), 35 deletions(-) create mode 100644 core/java/android/app/DexLoadReporter.java diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index c7fc860a055c6..a63fb8600fa16 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -143,6 +143,7 @@ import libcore.io.DropBox; import libcore.io.EventLogger; import libcore.io.IoUtils; import libcore.net.event.NetworkEventDispatcher; +import dalvik.system.BaseDexClassLoader; import dalvik.system.CloseGuard; import dalvik.system.VMDebug; import dalvik.system.VMRuntime; @@ -5346,6 +5347,16 @@ public final class ActivityThread { } } + // If we use profiles, setup the dex reporter to notify package manager + // of any relevant dex loads. The idle maintenance job will use the information + // reported to optimize the loaded dex files. + // Note that we only need one global reporter per app. + // Make sure we do this before calling onCreate so that we can capture the + // complete application startup. + if (SystemProperties.getBoolean("dalvik.vm.usejitprofiles", false)) { + BaseDexClassLoader.setReporter(DexLoadReporter.INSTANCE); + } + // Install the Network Security Config Provider. This must happen before the application // code is loaded to prevent issues with instances of TLS objects being created before // the provider is installed. diff --git a/core/java/android/app/DexLoadReporter.java b/core/java/android/app/DexLoadReporter.java new file mode 100644 index 0000000000000..009c44833e570 --- /dev/null +++ b/core/java/android/app/DexLoadReporter.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2017 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.app; + +import android.os.RemoteException; +import android.util.Slog; + +import dalvik.system.BaseDexClassLoader; +import dalvik.system.VMRuntime; + +import java.util.List; + +/** + * A dex load reporter which will notify package manager of any dex file loaded + * with {@code BaseDexClassLoader}. + * The goals are: + * 1) discover secondary dex files so that they can be optimized during the + * idle maintenance job. + * 2) determine whether or not a dex file is used by an app which does not + * own it (in order to select the optimal compilation method). + * @hide + */ +/*package*/ class DexLoadReporter implements BaseDexClassLoader.Reporter { + private static final String TAG = "DexLoadReporter"; + + /*package*/ static final DexLoadReporter INSTANCE = new DexLoadReporter(); + + private DexLoadReporter() {} + + @Override + public void report(List dexPaths) { + if (dexPaths.isEmpty()) { + return; + } + String packageName = ActivityThread.currentPackageName(); + try { + ActivityThread.getPackageManager().notifyDexLoad( + packageName, dexPaths, VMRuntime.getRuntime().vmInstructionSet()); + } catch (RemoteException re) { + Slog.e(TAG, "Failed to notify PM about dex load for package " + packageName, re); + } + } +} diff --git a/core/java/android/app/LoadedApk.java b/core/java/android/app/LoadedApk.java index c625756cac1cf..d3f57a183da13 100644 --- a/core/java/android/app/LoadedApk.java +++ b/core/java/android/app/LoadedApk.java @@ -51,7 +51,6 @@ import android.util.SparseArray; import android.view.Display; import android.view.DisplayAdjustments; -import dalvik.system.BaseDexClassLoader; import dalvik.system.VMRuntime; import java.io.File; @@ -609,40 +608,6 @@ public final class LoadedApk { VMRuntime.registerAppInfo(profileFile.getPath(), codePaths.toArray(new String[codePaths.size()])); - - // Setup the reporter to notify package manager of any relevant dex loads. - // At this point the primary apk is loaded and will not be reported. - // Anything loaded from now on will be tracked as a potential secondary - // or foreign dex file. The goal is to enable: - // 1) monitoring and compilation of secondary dex file - // 2) track whether or not a dex file is used by other apps (used to - // determined the compilation filter of apks). - if (BaseDexClassLoader.getReporter() != DexLoadReporter.INSTANCE) { - // Set the dex load reporter if not already set. - // Note that during the app's life cycle different LoadedApks may be - // created and loaded (e.g. if two different apps share the same runtime). - BaseDexClassLoader.setReporter(DexLoadReporter.INSTANCE); - } - } - - private static class DexLoadReporter implements BaseDexClassLoader.Reporter { - private static final DexLoadReporter INSTANCE = new DexLoadReporter(); - - private DexLoadReporter() {} - - @Override - public void report(List dexPaths) { - if (dexPaths.isEmpty()) { - return; - } - String packageName = ActivityThread.currentPackageName(); - try { - ActivityThread.getPackageManager().notifyDexLoad( - packageName, dexPaths, VMRuntime.getRuntime().vmInstructionSet()); - } catch (RemoteException re) { - Slog.e(TAG, "Failed to notify PM about dex load for package " + packageName, re); - } - } } /** From a9f46f76afd59dd502a93126989579c860df42b2 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Mon, 13 Mar 2017 23:30:30 -0700 Subject: [PATCH 5/6] Register secondary dex files for JIT profiling Test: boot, and check that profiles get recorded for secondary dex files Bug: 32871170 Bug: 26719109 (cherry picked from commit f5a7bfc8d5f9e74d6fa1e5be3246e7b4cde65bf6) Merged-In: I2de23ef44eee3f1783ae698821f1c6d88c66c9a6 Change-Id: Id9bbd630b8485dc17eeef846295458df5cfe446a --- core/java/android/app/ActivityThread.java | 2 +- core/java/android/app/DexLoadReporter.java | 115 ++++++++++++++++++++- core/java/android/app/LoadedApk.java | 5 + core/java/android/os/FileUtils.java | 7 +- 4 files changed, 122 insertions(+), 7 deletions(-) diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index a63fb8600fa16..f2721053bb054 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -5354,7 +5354,7 @@ public final class ActivityThread { // Make sure we do this before calling onCreate so that we can capture the // complete application startup. if (SystemProperties.getBoolean("dalvik.vm.usejitprofiles", false)) { - BaseDexClassLoader.setReporter(DexLoadReporter.INSTANCE); + BaseDexClassLoader.setReporter(DexLoadReporter.getInstance()); } // Install the Network Security Config Provider. This must happen before the application diff --git a/core/java/android/app/DexLoadReporter.java b/core/java/android/app/DexLoadReporter.java index 009c44833e570..13f288ab74549 100644 --- a/core/java/android/app/DexLoadReporter.java +++ b/core/java/android/app/DexLoadReporter.java @@ -16,13 +16,21 @@ package android.app; +import android.os.FileUtils; import android.os.RemoteException; +import android.os.SystemProperties; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; + import dalvik.system.BaseDexClassLoader; import dalvik.system.VMRuntime; +import java.io.File; +import java.io.IOException; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * A dex load reporter which will notify package manager of any dex file loaded @@ -37,15 +45,60 @@ import java.util.List; /*package*/ class DexLoadReporter implements BaseDexClassLoader.Reporter { private static final String TAG = "DexLoadReporter"; - /*package*/ static final DexLoadReporter INSTANCE = new DexLoadReporter(); + private static final DexLoadReporter INSTANCE = new DexLoadReporter(); - private DexLoadReporter() {} + private static final boolean DEBUG = false; + + // We must guard the access to the list of data directories because + // we might have concurrent accesses. Apps might load dex files while + // new data dirs are registered (due to creation of LoadedApks via + // create createApplicationContext). + @GuardedBy("mDataDirs") + private final Set mDataDirs; + + private DexLoadReporter() { + mDataDirs = new HashSet<>(); + } + + /*package*/ static DexLoadReporter getInstance() { + return INSTANCE; + } + + /** + * Register an application data directory with the reporter. + * The data directories are used to determine if a dex file is secondary dex or not. + * Note that this method may be called multiple times for the same app, registering + * different data directories. This may happen when apps share the same user id + * ({@code android:sharedUserId}). For example, if app1 and app2 share the same user + * id, and app1 loads app2 apk, then both data directories will be registered. + */ + /*package*/ void registerAppDataDir(String packageName, String dataDir) { + if (DEBUG) { + Slog.i(TAG, "Package " + packageName + " registering data dir: " + dataDir); + } + // TODO(calin): A few code paths imply that the data dir + // might be null. Investigate when that can happen. + if (dataDir != null) { + synchronized (mDataDirs) { + mDataDirs.add(dataDir); + } + } + } @Override public void report(List dexPaths) { if (dexPaths.isEmpty()) { return; } + // Notify the package manager about the dex loads unconditionally. + // The load might be for either a primary or secondary dex file. + notifyPackageManager(dexPaths); + // Check for secondary dex files and register them for profiling if + // possible. + registerSecondaryDexForProfiling(dexPaths); + } + + private void notifyPackageManager(List dexPaths) { String packageName = ActivityThread.currentPackageName(); try { ActivityThread.getPackageManager().notifyDexLoad( @@ -54,4 +107,62 @@ import java.util.List; Slog.e(TAG, "Failed to notify PM about dex load for package " + packageName, re); } } + + private void registerSecondaryDexForProfiling(List dexPaths) { + if (!SystemProperties.getBoolean("dalvik.vm.dexopt.secondary", false)) { + return; + } + // Make a copy of the current data directories so that we don't keep the lock + // while registering for profiling. The registration will perform I/O to + // check for or create the profile. + String[] dataDirs; + synchronized (mDataDirs) { + dataDirs = mDataDirs.toArray(new String[0]); + } + for (String dexPath : dexPaths) { + registerSecondaryDexForProfiling(dexPath, dataDirs); + } + } + + private void registerSecondaryDexForProfiling(String dexPath, String[] dataDirs) { + if (!isSecondaryDexFile(dexPath, dataDirs)) { + // The dex path is not a secondary dex file. Nothing to do. + return; + } + File secondaryProfile = getSecondaryProfileFile(dexPath); + try { + // Create the profile if not already there. + // Returns true if the file was created, false if the file already exists. + // or throws exceptions in case of errors. + boolean created = secondaryProfile.createNewFile(); + if (DEBUG && created) { + Slog.i(TAG, "Created profile for secondary dex: " + secondaryProfile); + } + } catch (IOException ex) { + Slog.e(TAG, "Failed to create profile for secondary dex " + secondaryProfile + + ":" + ex.getMessage()); + // Don't move forward with the registration if we failed to create the profile. + return; + } + + VMRuntime.registerAppInfo(secondaryProfile.getPath(), new String[] { dexPath }); + } + + // A dex file is a secondary dex file if it is in any of the registered app + // data directories. + private boolean isSecondaryDexFile(String dexPath, String[] dataDirs) { + for (String dataDir : dataDirs) { + if (FileUtils.contains(dataDir, dexPath)) { + return true; + } + } + return false; + } + + // Secondary dex profiles are stored next to the dex file and have the same + // name with '.prof' appended. + // NOTE: Keep in sync with installd. + private File getSecondaryProfileFile(String dexPath) { + return new File(dexPath + ".prof"); + } } diff --git a/core/java/android/app/LoadedApk.java b/core/java/android/app/LoadedApk.java index d3f57a183da13..404b7db8fe327 100644 --- a/core/java/android/app/LoadedApk.java +++ b/core/java/android/app/LoadedApk.java @@ -608,6 +608,11 @@ public final class LoadedApk { VMRuntime.registerAppInfo(profileFile.getPath(), codePaths.toArray(new String[codePaths.size()])); + + // Register the app data directory with the reporter. It will + // help deciding whether or not a dex file is the primary apk or a + // secondary dex. + DexLoadReporter.getInstance().registerAppDataDir(mPackageName, mDataDir); } /** diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java index 8e24caf478393..bd8af4deec9a6 100644 --- a/core/java/android/os/FileUtils.java +++ b/core/java/android/os/FileUtils.java @@ -428,14 +428,13 @@ public class FileUtils { */ public static boolean contains(File dir, File file) { if (dir == null || file == null) return false; + return contains(dir.getAbsolutePath(), file.getAbsolutePath()); + } - String dirPath = dir.getAbsolutePath(); - String filePath = file.getAbsolutePath(); - + public static boolean contains(String dirPath, String filePath) { if (dirPath.equals(filePath)) { return true; } - if (!dirPath.endsWith("/")) { dirPath += "/"; } From c480bdafdb984f024bb13a8f0de9b6e1efd34bcd Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 9 Mar 2017 18:41:10 -0800 Subject: [PATCH 6/6] Compile secondary dex files according to REASON_BACKGROUND_DEXOPT Test: adb shell cmd package bg-dexopt-job works for sercondary dex files Bug: 32871170 Bug: 26719109 (cherry picked from commit a70b1b120365976f5eeb7cb3f577f5f6ff7ad18e) Merged-In: Iaee8071449c83e774b2fe331aaccfc5433856f4f Change-Id: I2334e4d73a5acdd1106e913c59ddf642782f1eee --- .../android/server/pm/BackgroundDexOptService.java | 4 ++-- .../com/android/server/pm/PackageManagerService.java | 5 +++++ .../java/com/android/server/pm/dex/DexManager.java | 11 +++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/pm/BackgroundDexOptService.java b/services/core/java/com/android/server/pm/BackgroundDexOptService.java index d8900c0546fd6..d364d17fa764d 100644 --- a/services/core/java/com/android/server/pm/BackgroundDexOptService.java +++ b/services/core/java/com/android/server/pm/BackgroundDexOptService.java @@ -290,8 +290,8 @@ public class BackgroundDexOptService extends JobService { PackageManagerService.REASON_BACKGROUND_DEXOPT, /* force */ false) : pm.performDexOptSecondary(pkg, - PackageManagerServiceCompilerMapping.getFullCompilerFilter(), - /* force */ true); + PackageManagerService.REASON_BACKGROUND_DEXOPT, + /* force */ false); if (success) { // Dexopt succeeded, remove package from the list of failing ones. synchronized (failedPackageNames) { diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 5945ed04e321e..2b7e9b9617b16 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -7516,6 +7516,11 @@ public class PackageManagerService extends IPackageManager.Stub { return mDexManager.dexoptSecondaryDex(packageName, compilerFilter, force); } + public boolean performDexOptSecondary(String packageName, int compileReason, + boolean force) { + return mDexManager.dexoptSecondaryDex(packageName, compileReason, force); + } + /** * Reconcile the information we have about the secondary dex files belonging to * {@code packagName} and the actual dex files. For all dex files that were diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 83dd392988bfc..a904d178b03d6 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -274,6 +274,17 @@ public class DexManager { return mPackageDexUsage.getPackageUseInfo(packageName); } + /** + * Perform dexopt on the package {@code packageName} secondary dex files. + * @return true if all secondary dex files were processed successfully (compiled or skipped + * because they don't need to be compiled).. + */ + public boolean dexoptSecondaryDex(String packageName, int compilerReason, boolean force) { + return dexoptSecondaryDex(packageName, + PackageManagerServiceCompilerMapping.getCompilerFilterForReason(compilerReason), + force); + } + /** * Perform dexopt on the package {@code packageName} secondary dex files. * @return true if all secondary dex files were processed successfully (compiled or skipped