From 33fc19ce557ffbb7e451c2f89749c1842c88dc9c Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Sun, 29 Mar 2020 16:53:21 -0700 Subject: Remove support for PackageDexUsage version 1 PackageVersion version 1 was indroduce in Nougat. Since then, all devices should have updated to version 2 which was introduced in O. This simplifies the logic in preparation for adding support for system server jars. Test: atest DexManagerTests PackageDexUsageTests Bug: 148774920 Change-Id: Ib546ed12c5341d01fb12545cd98598b4cf2c5051 --- .../com/android/server/pm/PackageDexOptimizer.java | 3 +- .../java/com/android/server/pm/dex/DexManager.java | 2 +- .../com/android/server/pm/dex/PackageDexUsage.java | 161 ++++++--------------- .../server/pm/dex/PackageDexUsageTests.java | 104 +------------ 4 files changed, 51 insertions(+), 219 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageDexOptimizer.java b/services/core/java/com/android/server/pm/PackageDexOptimizer.java index e625aeffc0c6..e7d0c41c0fea 100644 --- a/services/core/java/com/android/server/pm/PackageDexOptimizer.java +++ b/services/core/java/com/android/server/pm/PackageDexOptimizer.java @@ -401,7 +401,8 @@ public class PackageDexOptimizer { return DEX_OPT_FAILED; } String classLoaderContext = null; - if (dexUseInfo.isUnknownClassLoaderContext() || dexUseInfo.isVariableClassLoaderContext()) { + if (dexUseInfo.isUnsupportedClassLoaderContext() + || dexUseInfo.isVariableClassLoaderContext()) { // If we have an unknown (not yet set), or a variable class loader chain. Just extract // the dex file. compilerFilter = "extract"; 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 117cc5e8eb80..5ba1996ed3c9 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -563,7 +563,7 @@ public class DexManager { boolean newUpdate = mPackageDexUsage.record(searchResult.mOwningPackageName, dexPath, userId, isa, isUsedByOtherApps, /*primaryOrSplit*/ false, searchResult.mOwningPackageName, - PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT); + PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT); update |= newUpdate; } if (update) { 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 08763e729c71..8819a0fe439b 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -51,25 +51,21 @@ import java.util.Set; * Stat file which store usage information about dex files. */ public class PackageDexUsage extends AbstractStatsBase { - private final static String TAG = "PackageDexUsage"; + private static final String TAG = "PackageDexUsage"; - // We support previous version to ensure that the usage list remains valid cross OTAs. - private final static int PACKAGE_DEX_USAGE_SUPPORTED_VERSION_1 = 1; - // Version 2 added: - // - the list of packages that load the dex files - // - class loader contexts for secondary dex files - // - usage for all code paths (including splits) - private final static int PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2 = 2; + // We are currently at version 2. + // Version 1 was introduced in Nougat and Version 2 in Oreo. + // We dropped version 1 support in R since all devices should have updated + // already. + private static final int PACKAGE_DEX_USAGE_VERSION = 2; - private final static int PACKAGE_DEX_USAGE_VERSION = PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2; - - private final static String PACKAGE_DEX_USAGE_VERSION_HEADER = + private static final String PACKAGE_DEX_USAGE_VERSION_HEADER = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__"; - private final static String SPLIT_CHAR = ","; - private final static String CODE_PATH_LINE_CHAR = "+"; - private final static String DEX_LINE_CHAR = "#"; - private final static String LOADING_PACKAGE_CHAR = "@"; + private static final String SPLIT_CHAR = ","; + private static final String CODE_PATH_LINE_CHAR = "+"; + private static final String DEX_LINE_CHAR = "#"; + private static final String LOADING_PACKAGE_CHAR = "@"; // One of the things we record about dex files is the class loader context that was used to // load them. That should be stable but if it changes we don't keep track of variable contexts. @@ -77,10 +73,6 @@ public class PackageDexUsage extends AbstractStatsBase { // skip optimizations on that dex files. /*package*/ static final String VARIABLE_CLASS_LOADER_CONTEXT = "=VariableClassLoaderContext="; - // The markers used for unknown class loader contexts. This can happen if the dex file was - // recorded in a previous version and we didn't have a chance to update its usage. - /*package*/ static final String UNKNOWN_CLASS_LOADER_CONTEXT = - "=UnknownClassLoaderContext="; // The marker used for unsupported class loader contexts (no longer written, may occur in old // files so discarded on read). Note: this matches @@ -339,7 +331,9 @@ public class PackageDexUsage extends AbstractStatsBase { version = Integer.parseInt( versionLine.substring(PACKAGE_DEX_USAGE_VERSION_HEADER.length())); if (!isSupportedVersion(version)) { - throw new IllegalStateException("Unexpected version: " + version); + Slog.w(TAG, "Unexpected package-dex-use version: " + version + + ". Not reading from it"); + return; } } @@ -377,9 +371,8 @@ public class PackageDexUsage extends AbstractStatsBase { throw new IllegalStateException("Invalid PackageDexUsage line: " + line); } - // In version 2 we added the loading packages and class loader context. - Set loadingPackages = maybeReadLoadingPackages(in, version); - String classLoaderContext = maybeReadClassLoaderContext(in, version); + Set loadingPackages = readLoadingPackages(in, version); + String classLoaderContext = readClassLoaderContext(in, version); if (UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(classLoaderContext)) { // We used to record use of unsupported class loaders, but we no longer do. @@ -410,34 +403,16 @@ public class PackageDexUsage extends AbstractStatsBase { } currentPackageData.mDexUseInfoMap.put(dexPath, dexUseInfo); } else if (line.startsWith(CODE_PATH_LINE_CHAR)) { - // This is a code path used by other apps line. - if (version < PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2) { - throw new IllegalArgumentException("Unexpected code path line when parsing " + - "PackageDexUseData: " + line); - } - // Expects 2 lines: // +code_paths // @loading_packages String codePath = line.substring(CODE_PATH_LINE_CHAR.length()); - Set loadingPackages = maybeReadLoadingPackages(in, version); + Set loadingPackages = readLoadingPackages(in, version); currentPackageData.mCodePathsUsedByOtherApps.put(codePath, loadingPackages); } else { // This is a package line. - if (version >= PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2) { - currentPackage = line; - currentPackageData = new PackageUseInfo(); - } else { - // Old version (<2) - // We expect it to be: `packageName,isUsedByOtherApps`. - String[] elems = line.split(SPLIT_CHAR); - if (elems.length != 2) { - throw new IllegalStateException("Invalid PackageDexUsage line: " + line); - } - currentPackage = elems[0]; - currentPackageData = new PackageUseInfo(); - currentPackageData.mUsedByOtherAppsBeforeUpgrade = readBoolean(elems[1]); - } + currentPackage = line; + currentPackageData = new PackageUseInfo(); data.put(currentPackage, currentPackageData); } } @@ -449,45 +424,33 @@ public class PackageDexUsage extends AbstractStatsBase { } /** - * Reads the class loader context encoding from the buffer {@code in} if - * {@code version} is at least {PACKAGE_DEX_USAGE_VERSION}. + * Reads the class loader context encoding from the buffer {@code in}. */ - private String maybeReadClassLoaderContext(BufferedReader in, int version) throws IOException { - String context = null; - if (version >= PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2) { - context = in.readLine(); - if (context == null) { - throw new IllegalStateException("Could not find the classLoaderContext line."); - } + private String readClassLoaderContext(BufferedReader in, int version) throws IOException { + String context = in.readLine(); + if (context == null) { + throw new IllegalStateException("Could not find the classLoaderContext line."); } - // The context might be empty if we didn't have the chance to update it after a version - // upgrade. In this case return the special marker so that we recognize this is an unknown - // context. - return context == null ? UNKNOWN_CLASS_LOADER_CONTEXT : context; + return context; } /** - * Reads the list of loading packages from the buffer {@code in} if - * {@code version} is at least {PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2}. + * Reads the list of loading packages from the buffer {@code in}. */ - private Set maybeReadLoadingPackages(BufferedReader in, int version) + private Set readLoadingPackages(BufferedReader in, int version) throws IOException { - if (version >= PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2) { - String line = in.readLine(); - if (line == null) { - throw new IllegalStateException("Could not find the loadingPackages line."); - } - // We expect that most of the times the list of loading packages will be empty. - if (line.length() == LOADING_PACKAGE_CHAR.length()) { - return Collections.emptySet(); - } else { - Set result = new HashSet<>(); - Collections.addAll(result, - line.substring(LOADING_PACKAGE_CHAR.length()).split(SPLIT_CHAR)); - return result; - } - } else { + String line = in.readLine(); + if (line == null) { + throw new IllegalStateException("Could not find the loadingPackages line."); + } + // We expect that most of the times the list of loading packages will be empty. + if (line.length() == LOADING_PACKAGE_CHAR.length()) { return Collections.emptySet(); + } else { + Set result = new HashSet<>(); + Collections.addAll(result, + line.substring(LOADING_PACKAGE_CHAR.length()).split(SPLIT_CHAR)); + return result; } } @@ -501,8 +464,7 @@ public class PackageDexUsage extends AbstractStatsBase { } private boolean isSupportedVersion(int version) { - return version == PACKAGE_DEX_USAGE_SUPPORTED_VERSION_1 - || version == PACKAGE_DEX_USAGE_SUPPORTED_VERSION_2; + return version == PACKAGE_DEX_USAGE_VERSION; } /** @@ -544,14 +506,7 @@ public class PackageDexUsage extends AbstractStatsBase { } } - // In case the package was marked as used by other apps in a previous version - // propagate the flag to all the code paths. - // See mUsedByOtherAppsBeforeUpgrade docs on why it is important to do it. - if (packageUseInfo.mUsedByOtherAppsBeforeUpgrade) { - for (String codePath : codePaths) { - packageUseInfo.mergeCodePathUsedByOtherApps(codePath, true, null, null); - } - } else if (!packageUseInfo.isAnyCodePathUsedByOtherApps() + if (!packageUseInfo.isAnyCodePathUsedByOtherApps() && packageUseInfo.mDexUseInfoMap.isEmpty()) { // The package is not used by other apps and we removed all its dex files // records. Remove the entire package record as well. @@ -718,19 +673,6 @@ public class PackageDexUsage extends AbstractStatsBase { // Map dex paths to their data (isUsedByOtherApps, owner id, loader isa). private final Map mDexUseInfoMap; - // Keeps track of whether or not this package was used by other apps before - // we upgraded to VERSION 4 which records the info for each code path separately. - // This is unwanted complexity but without it we risk to profile guide compile - // something that supposed to be shared. For example: - // 1) we determine that chrome is used by another app - // 2) we take an OTA which upgrades the way we keep track of usage data - // 3) chrome doesn't get used until the background job executes - // 4) as part of the backgound job we now think that chrome is not used by others - // and we speed-profile. - // 5) as a result the next time someone uses chrome it will extract from apk since - // the compiled code will be private. - private boolean mUsedByOtherAppsBeforeUpgrade; - /*package*/ PackageUseInfo() { mCodePathsUsedByOtherApps = new HashMap<>(); mDexUseInfoMap = new HashMap<>(); @@ -790,10 +732,6 @@ public class PackageDexUsage extends AbstractStatsBase { * Returns whether or not there was an update. */ /*package*/ boolean clearCodePathUsedByOtherApps() { - // Update mUsedByOtherAppsBeforeUpgrade as well to be consistent with - // the new data. This is not saved to disk so we don't need to return it. - mUsedByOtherAppsBeforeUpgrade = true; - if (mCodePathsUsedByOtherApps.isEmpty()) { return false; } else { @@ -847,11 +785,9 @@ public class PackageDexUsage extends AbstractStatsBase { boolean updateLoadingPackages = mLoadingPackages.addAll(dexUseInfo.mLoadingPackages); String oldClassLoaderContext = mClassLoaderContext; - if (isUnknownOrUnsupportedContext(mClassLoaderContext)) { - // Can happen if we read a previous version. + if (isUnsupportedContext(mClassLoaderContext)) { mClassLoaderContext = dexUseInfo.mClassLoaderContext; - } else if (!isUnknownOrUnsupportedContext(dexUseInfo.mClassLoaderContext) - && !Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) { + } else if (!Objects.equals(mClassLoaderContext, dexUseInfo.mClassLoaderContext)) { // We detected a context change. mClassLoaderContext = VARIABLE_CLASS_LOADER_CONTEXT; } @@ -862,11 +798,8 @@ public class PackageDexUsage extends AbstractStatsBase { || !Objects.equals(oldClassLoaderContext, mClassLoaderContext); } - private static boolean isUnknownOrUnsupportedContext(String context) { - // TODO: Merge UNKNOWN_CLASS_LOADER_CONTEXT & UNSUPPORTED_CLASS_LOADER_CONTEXT cases - // into UNSUPPORTED_CLASS_LOADER_CONTEXT. - return UNKNOWN_CLASS_LOADER_CONTEXT.equals(context) - || UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(context); + private static boolean isUnsupportedContext(String context) { + return UNSUPPORTED_CLASS_LOADER_CONTEXT.equals(context); } public boolean isUsedByOtherApps() { @@ -887,10 +820,8 @@ public class PackageDexUsage extends AbstractStatsBase { public String getClassLoaderContext() { return mClassLoaderContext; } - public boolean isUnknownClassLoaderContext() { - // The class loader context may be unknown if we loaded the data from a previous version - // which didn't save the context. - return isUnknownOrUnsupportedContext(mClassLoaderContext); + public boolean isUnsupportedClassLoaderContext() { + return isUnsupportedContext(mClassLoaderContext); } public boolean isVariableClassLoaderContext() { 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 5df4509af885..f01ddfca15c6 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 @@ -420,45 +420,19 @@ public class PackageDexUsageTests { assertPackageDexUsage(null, expectedContext); } - @Test - public void testRecordClassLoaderContextTransitionFromUnknown() { - // Record a secondary dex file. - TestData unknownContext = mFooSecondary1User0.updateClassLoaderContext( - PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT); - assertTrue(record(unknownContext)); - - assertPackageDexUsage(null, unknownContext); - writeAndReadBack(); - assertPackageDexUsage(null, unknownContext); - - // Now update the secondary dex record with a class loader context. This simulates the - // version 2 to version 3 upgrade. - - assertTrue(record(mFooSecondary1User0)); - - assertPackageDexUsage(null, mFooSecondary1User0); - writeAndReadBack(); - assertPackageDexUsage(null, mFooSecondary1User0); - } - @Test public void testDexUsageClassLoaderContext() { final boolean isUsedByOtherApps = false; final int userId = 0; PackageDexUsage.DexUseInfo validContext = new DexUseInfo(isUsedByOtherApps, userId, "valid_context", "arm"); - assertFalse(validContext.isUnknownClassLoaderContext()); + assertFalse(validContext.isUnsupportedClassLoaderContext()); assertFalse(validContext.isVariableClassLoaderContext()); PackageDexUsage.DexUseInfo variableContext = new DexUseInfo(isUsedByOtherApps, userId, PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, "arm"); - assertFalse(variableContext.isUnknownClassLoaderContext()); + assertFalse(variableContext.isUnsupportedClassLoaderContext()); assertTrue(variableContext.isVariableClassLoaderContext()); - - PackageDexUsage.DexUseInfo unknownContext = new DexUseInfo(isUsedByOtherApps, userId, - PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT, "arm"); - assertTrue(unknownContext.isUnknownClassLoaderContext()); - assertFalse(unknownContext.isVariableClassLoaderContext()); } @Test @@ -482,80 +456,6 @@ public class PackageDexUsageTests { assertPackageDexUsage(mBarBaseUser0); } - @Test - public void testReadVersion1() { - String isa = VMRuntime.getInstructionSet(Build.SUPPORTED_ABIS[0]); - // Equivalent to - // record(mFooSplit2UsedByOtherApps0); - // record(mFooSecondary1User0); - // record(mFooSecondary2UsedByOtherApps0); - // record(mBarBaseUser0); - // record(mBarSecondary1User0); - String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__1\n" - + "com.google.foo,1\n" - + "#/data/user/0/com.google.foo/sec-1.dex\n" - + "0,0," + isa + "\n" - + "#/data/user/0/com.google.foo/sec-2.dex\n" - + "0,1," + isa + "\n" - + "com.google.bar,0\n" - + "#/data/user/0/com.google.bar/sec-1.dex\n" - + "0,0," + isa + "\n"; - - PackageDexUsage packageDexUsage = new PackageDexUsage(); - try { - packageDexUsage.read(new StringReader(content)); - } catch (IOException e) { - fail(); - } - - // After the read we must sync the data to fill the missing information on the code paths. - Map> packageToUsersMap = new HashMap<>(); - Map> packageToCodePaths = new HashMap<>(); - - // Handle foo package. - packageToUsersMap.put(mFooSplit2UsedByOtherApps0.mPackageName, - new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mOwnerUserId))); - packageToCodePaths.put(mFooSplit2UsedByOtherApps0.mPackageName, - new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mDexFile, - mFooSplit1User0.mDexFile, mFooBaseUser0.mDexFile))); - // Handle bar package. - packageToUsersMap.put(mBarBaseUser0.mPackageName, - new HashSet<>(Arrays.asList(mBarBaseUser0.mOwnerUserId))); - packageToCodePaths.put(mBarBaseUser0.mPackageName, - new HashSet<>(Arrays.asList(mBarBaseUser0.mDexFile))); - - // Sync the data. - packageDexUsage.syncData(packageToUsersMap, packageToCodePaths); - - // Update the class loaders to unknown before asserting if needed. Before version 2 we - // didn't have any. - String unknown = PackageDexUsage.UNKNOWN_CLASS_LOADER_CONTEXT; - TestData fooBaseUser0 = mFooBaseUser0.updateClassLoaderContext(unknown); - TestData fooSplit1User0 = mFooSplit1User0.updateClassLoaderContext(unknown); - TestData fooSplit2UsedByOtherApps0 = - mFooSplit2UsedByOtherApps0.updateClassLoaderContext(unknown); - TestData fooSecondary1User0 = mFooSecondary1User0.updateClassLoaderContext(unknown); - TestData fooSecondary2UsedByOtherApps0 = - mFooSecondary2UsedByOtherApps0.updateClassLoaderContext(unknown); - TestData barBaseUser0 = mBarBaseUser0.updateClassLoaderContext(unknown); - TestData barSecondary1User0 = mBarSecondary1User0.updateClassLoaderContext(unknown); - - // Assert foo code paths. Note that we ignore the users during upgrade. - final Set ignoredUsers = null; - assertPackageDexUsage(packageDexUsage, ignoredUsers, - fooSplit2UsedByOtherApps0, fooSecondary1User0, fooSecondary2UsedByOtherApps0); - // Because fooSplit2UsedByOtherApps0 is used by others, all the other code paths must - // share the same data. - assertPackageDexUsage(packageDexUsage, ignoredUsers, - fooSplit1User0.updateUseByOthers(true), - fooSecondary1User0, fooSecondary2UsedByOtherApps0); - assertPackageDexUsage(packageDexUsage, ignoredUsers, fooBaseUser0.updateUseByOthers(true), - fooSecondary1User0, fooSecondary2UsedByOtherApps0); - - // Assert bar code paths. Note that we ignore the users during upgrade. - assertPackageDexUsage(packageDexUsage, ignoredUsers, barBaseUser0, barSecondary1User0); - } - private void assertPackageDexUsage(TestData primary, TestData... secondaries) { assertPackageDexUsage(mPackageDexUsage, null, primary, secondaries); } -- cgit v1.2.3-59-g8ed1b From 3f2b728bba995f99b3518713eb4ba3c53644648b Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Sun, 29 Mar 2020 17:14:59 -0700 Subject: Allow PackageDexUsage to record code paths not used by others The restriction was just an optimization to make the processing simpler and clearer. All packages are expected to use their code paths and recording this info was redundant and added noise. However, in prepartion to record system server code paths we have to record everything that belongs to a package, even if it's not used by others. This CL makes PackageDexUsage agnostic of the isUsedByOther flags: it will record everything that is given to it. Note that this applies only to PackageDexUsage. DexManager may still decide to skip the recording (which it does for non system server apps) Test: atest PackageDexUsageTests DexManagerTests Bug: 148774920 Change-Id: Icdb9ff45330a8fb15d80cb07495723b473228dda --- .../java/com/android/server/pm/dex/DexManager.java | 35 ++-- .../com/android/server/pm/dex/PackageDexUsage.java | 92 ++++++----- .../server/pm/dex/PackageDexUsageTests.java | 182 ++++++++++++++++----- 3 files changed, 211 insertions(+), 98 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 5ba1996ed3c9..9f2ec47335f3 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -103,19 +103,6 @@ public class DexManager { private static int DEX_SEARCH_FOUND_SPLIT = 2; // dex file is a split apk private static int DEX_SEARCH_FOUND_SECONDARY = 3; // dex file is a secondary dex - /** - * We do not record packages that have no secondary dex files or that are not used by other - * apps. This is an optimization to reduce the amount of data that needs to be written to - * disk (apps will not usually be shared so this trims quite a bit the number we record). - * - * To make this behaviour transparent to the callers which need use information on packages, - * DexManager will return this DEFAULT instance from - * {@link DexManager#getPackageUseInfoOrDefault}. It has no data about secondary dex files and - * is marked as not being used by other apps. This reflects the intended behaviour when we don't - * find the package in the underlying data file. - */ - private final static PackageUseInfo DEFAULT_USE_INFO = new PackageUseInfo(); - public DexManager(Context context, IPackageManager pms, PackageDexOptimizer pdo, Installer installer, Object installLock) { mContext = context; @@ -194,6 +181,8 @@ public class DexManager { // If the dex file is the primary apk (or a split) and not isUsedByOtherApps // do not record it. This case does not bring any new usable information // and can be safely skipped. + // Note this is just an optimization that makes things easier to read in the + // package-dex-use file since we don't need to pollute it with redundant info. continue; } @@ -211,7 +200,7 @@ public class DexManager { // async write to disk to make sure we don't loose the data in case of a reboot. if (mPackageDexUsage.record(searchResult.mOwningPackageName, - dexPath, loaderUserId, loaderIsa, isUsedByOtherApps, primaryOrSplit, + dexPath, loaderUserId, loaderIsa, primaryOrSplit, loadingAppInfo.packageName, classLoaderContext)) { mPackageDexUsage.maybeWriteAsync(); } @@ -391,8 +380,17 @@ public class DexManager { * to access the package use. */ public PackageUseInfo getPackageUseInfoOrDefault(String packageName) { + // We do not record packages that have no secondary dex files or that are not used by other + // apps. This is an optimization to reduce the amount of data that needs to be written to + // disk (apps will not usually be shared so this trims quite a bit the number we record). + // + // To make this behaviour transparent to the callers which need use information on packages, + // DexManager will return this DEFAULT instance from + // {@link DexManager#getPackageUseInfoOrDefault}. It has no data about secondary dex files + // and is marked as not being used by other apps. This reflects the intended behaviour when + // we don't find the package in the underlying data file. PackageUseInfo useInfo = mPackageDexUsage.getPackageUseInfo(packageName); - return useInfo == null ? DEFAULT_USE_INFO : useInfo; + return useInfo == null ? new PackageUseInfo(packageName) : useInfo; } /** @@ -542,7 +540,7 @@ public class DexManager { // TODO(calin): questionable API in the presence of class loaders context. Needs amends as the // compilation happening here will use a pessimistic context. public RegisterDexModuleResult registerDexModule(ApplicationInfo info, String dexPath, - boolean isUsedByOtherApps, int userId) { + boolean isSharedModule, int userId) { // Find the owning package record. DexSearchResult searchResult = getDexPackage(info, dexPath, userId); @@ -559,9 +557,12 @@ public class DexManager { // We found the package. Now record the usage for all declared ISAs. boolean update = false; + // If this is a shared module set the loading package to an arbitrary package name + // so that we can mark that module as usedByOthers. + String loadingPackage = isSharedModule ? ".shared.module" : searchResult.mOwningPackageName; for (String isa : getAppDexInstructionSets(info.primaryCpuAbi, info.secondaryCpuAbi)) { boolean newUpdate = mPackageDexUsage.record(searchResult.mOwningPackageName, - dexPath, userId, isa, isUsedByOtherApps, /*primaryOrSplit*/ false, + dexPath, userId, isa, /*primaryOrSplit*/ false, searchResult.mOwningPackageName, PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT); update |= newUpdate; 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 8819a0fe439b..1a3c18f61333 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -39,10 +39,12 @@ import java.io.OutputStreamWriter; import java.io.Reader; import java.io.StringWriter; import java.io.Writer; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -118,7 +120,7 @@ public class PackageDexUsage extends AbstractStatsBase { * has been seen before. */ /* package */ boolean record(String owningPackageName, String dexPath, int ownerUserId, - String loaderIsa, boolean isUsedByOtherApps, boolean primaryOrSplit, + String loaderIsa, boolean primaryOrSplit, String loadingPackageName, String classLoaderContext) { if (!PackageManagerServiceUtils.checkISA(loaderIsa)) { throw new IllegalArgumentException("loaderIsa " + loaderIsa + " is unsupported"); @@ -127,20 +129,22 @@ public class PackageDexUsage extends AbstractStatsBase { throw new IllegalArgumentException("Null classLoaderContext"); } if (classLoaderContext.equals(UNSUPPORTED_CLASS_LOADER_CONTEXT)) { + Slog.e(TAG, "Unsupported context?"); return false; } + boolean isUsedByOtherApps = !owningPackageName.equals(loadingPackageName); + synchronized (mPackageUseInfoMap) { PackageUseInfo packageUseInfo = mPackageUseInfoMap.get(owningPackageName); if (packageUseInfo == null) { // This is the first time we see the package. - packageUseInfo = new PackageUseInfo(); + packageUseInfo = new PackageUseInfo(owningPackageName); if (primaryOrSplit) { // If we have a primary or a split apk, set isUsedByOtherApps. // We do not need to record the loaderIsa or the owner because we compile // primaries for all users and all ISAs. - packageUseInfo.mergeCodePathUsedByOtherApps(dexPath, isUsedByOtherApps, - owningPackageName, loadingPackageName); + packageUseInfo.mergePrimaryCodePaths(dexPath, loadingPackageName); } else { // For secondary dex files record the loaderISA and the owner. We'll need // to know under which user to compile and for what ISA. @@ -156,9 +160,8 @@ public class PackageDexUsage extends AbstractStatsBase { // We already have data on this package. Amend it. if (primaryOrSplit) { // We have a possible update on the primary apk usage. Merge - // isUsedByOtherApps information and return if there was an update. - return packageUseInfo.mergeCodePathUsedByOtherApps( - dexPath, isUsedByOtherApps, owningPackageName, loadingPackageName); + // dex path information and return if there was an update. + return packageUseInfo.mergePrimaryCodePaths(dexPath, loadingPackageName); } else { DexUseInfo newData = new DexUseInfo( isUsedByOtherApps, ownerUserId, classLoaderContext, loaderIsa); @@ -273,7 +276,7 @@ public class PackageDexUsage extends AbstractStatsBase { // Write the code paths used by other apps. for (Map.Entry> codeEntry : - packageUseInfo.mCodePathsUsedByOtherApps.entrySet()) { + packageUseInfo.mPrimaryCodePaths.entrySet()) { String codePath = codeEntry.getKey(); Set loadingPackages = codeEntry.getValue(); fpw.println(CODE_PATH_LINE_CHAR + codePath); @@ -408,11 +411,11 @@ public class PackageDexUsage extends AbstractStatsBase { // @loading_packages String codePath = line.substring(CODE_PATH_LINE_CHAR.length()); Set loadingPackages = readLoadingPackages(in, version); - currentPackageData.mCodePathsUsedByOtherApps.put(codePath, loadingPackages); + currentPackageData.mPrimaryCodePaths.put(codePath, loadingPackages); } else { // This is a package line. currentPackage = line; - currentPackageData = new PackageUseInfo(); + currentPackageData = new PackageUseInfo(currentPackage); data.put(currentPackage, currentPackageData); } } @@ -499,7 +502,7 @@ public class PackageDexUsage extends AbstractStatsBase { // Sync the code paths. Set codePaths = packageToCodePaths.get(packageName); Iterator>> codeIt = - packageUseInfo.mCodePathsUsedByOtherApps.entrySet().iterator(); + packageUseInfo.mPrimaryCodePaths.entrySet().iterator(); while (codeIt.hasNext()) { if (!codePaths.contains(codeIt.next().getKey())) { codeIt.remove(); @@ -667,22 +670,26 @@ public class PackageDexUsage extends AbstractStatsBase { * Stores data on how a package and its dex files are used. */ public static class PackageUseInfo { + // The name of the package this info belongs to. + private final String mPackageName; // The app's code paths that are used by other apps. // The key is the code path and the value is the set of loading packages. - private final Map> mCodePathsUsedByOtherApps; + private final Map> mPrimaryCodePaths; // Map dex paths to their data (isUsedByOtherApps, owner id, loader isa). private final Map mDexUseInfoMap; - /*package*/ PackageUseInfo() { - mCodePathsUsedByOtherApps = new HashMap<>(); + /*package*/ PackageUseInfo(String packageName) { + mPrimaryCodePaths = new HashMap<>(); mDexUseInfoMap = new HashMap<>(); + mPackageName = packageName; } // Creates a deep copy of the `other`. private PackageUseInfo(PackageUseInfo other) { - mCodePathsUsedByOtherApps = new HashMap<>(); - for (Map.Entry> e : other.mCodePathsUsedByOtherApps.entrySet()) { - mCodePathsUsedByOtherApps.put(e.getKey(), new HashSet<>(e.getValue())); + mPackageName = other.mPackageName; + mPrimaryCodePaths = new HashMap<>(); + for (Map.Entry> e : other.mPrimaryCodePaths.entrySet()) { + mPrimaryCodePaths.put(e.getKey(), new HashSet<>(e.getValue())); } mDexUseInfoMap = new HashMap<>(); @@ -691,28 +698,29 @@ public class PackageDexUsage extends AbstractStatsBase { } } - private boolean mergeCodePathUsedByOtherApps(String codePath, boolean isUsedByOtherApps, - String owningPackageName, String loadingPackage) { - if (!isUsedByOtherApps) { - // Nothing to update if the the code path is not used by other apps. - return false; - } - - boolean newCodePath = false; - Set loadingPackages = mCodePathsUsedByOtherApps.get(codePath); + private boolean mergePrimaryCodePaths(String codePath, String loadingPackage) { + Set loadingPackages = mPrimaryCodePaths.get(codePath); if (loadingPackages == null) { loadingPackages = new HashSet<>(); - mCodePathsUsedByOtherApps.put(codePath, loadingPackages); - newCodePath = true; + mPrimaryCodePaths.put(codePath, loadingPackages); } - boolean newLoadingPackage = loadingPackage != null - && !loadingPackage.equals(owningPackageName) - && loadingPackages.add(loadingPackage); - return newCodePath || newLoadingPackage; + return loadingPackages.add(loadingPackage); } public boolean isUsedByOtherApps(String codePath) { - return mCodePathsUsedByOtherApps.containsKey(codePath); + if (mPrimaryCodePaths.containsKey(codePath)) { + Set loadingPackages = mPrimaryCodePaths.get(codePath); + if (loadingPackages.contains(mPackageName)) { + // If the owning package is in the list then this code path + // is used by others if there are other packages in the list. + return loadingPackages.size() > 1; + } else { + // The owning package is not in the loading packages. So if + // the list is non-empty then the code path is used by others. + return !loadingPackages.isEmpty(); + } + } + return false; } public Map getDexUseInfoMap() { @@ -720,11 +728,11 @@ public class PackageDexUsage extends AbstractStatsBase { } public Set getLoadingPackages(String codePath) { - return mCodePathsUsedByOtherApps.getOrDefault(codePath, null); + return mPrimaryCodePaths.getOrDefault(codePath, null); } public boolean isAnyCodePathUsedByOtherApps() { - return !mCodePathsUsedByOtherApps.isEmpty(); + return !mPrimaryCodePaths.isEmpty(); } /** @@ -732,12 +740,16 @@ public class PackageDexUsage extends AbstractStatsBase { * Returns whether or not there was an update. */ /*package*/ boolean clearCodePathUsedByOtherApps() { - if (mCodePathsUsedByOtherApps.isEmpty()) { - return false; - } else { - mCodePathsUsedByOtherApps.clear(); - return true; + boolean updated = false; + List retainOnlyOwningPackage = new ArrayList<>(1); + retainOnlyOwningPackage.add(mPackageName); + for (Map.Entry> entry : mPrimaryCodePaths.entrySet()) { + // Remove or loading packages but the owning one. + if (entry.getValue().retainAll(retainOnlyOwningPackage)) { + updated = true; + } } + return updated; } } 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 f01ddfca15c6..08fc87758f57 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 @@ -77,25 +77,25 @@ public class PackageDexUsageTests { String fooDataDir = "/data/user/0/com.google.foo/"; mFooBaseUser0 = new TestData(fooPackageName, - fooCodeDir + "base.apk", 0, ISA, false, true, fooPackageName); + fooCodeDir + "base.apk", 0, ISA, true, fooPackageName); mFooSplit1User0 = new TestData(fooPackageName, - fooCodeDir + "split-1.apk", 0, ISA, false, true, fooPackageName); + fooCodeDir + "split-1.apk", 0, ISA, true, fooPackageName); mFooSplit2UsedByOtherApps0 = new TestData(fooPackageName, - fooCodeDir + "split-2.apk", 0, ISA, true, true, "used.by.other.com"); + fooCodeDir + "split-2.apk", 0, ISA, true, "used.by.other.com"); mFooSecondary1User0 = new TestData(fooPackageName, - fooDataDir + "sec-1.dex", 0, ISA, false, false, fooPackageName); + fooDataDir + "sec-1.dex", 0, ISA, false, fooPackageName); mFooSecondary1User1 = new TestData(fooPackageName, - fooDataDir + "sec-1.dex", 1, ISA, false, false, fooPackageName); + fooDataDir + "sec-1.dex", 1, ISA, false, fooPackageName); mFooSecondary2UsedByOtherApps0 = new TestData(fooPackageName, - fooDataDir + "sec-2.dex", 0, ISA, true, false, "used.by.other.com"); + fooDataDir + "sec-2.dex", 0, ISA, false, "used.by.other.com"); mInvalidIsa = new TestData(fooPackageName, - fooCodeDir + "base.apk", 0, "INVALID_ISA", false, true, "INALID_USER"); + fooCodeDir + "base.apk", 0, "INVALID_ISA", true, "INALID_USER"); String barPackageName = "com.google.bar"; String barCodeDir = "/data/app/com.google.bar/"; @@ -103,11 +103,11 @@ public class PackageDexUsageTests { String barDataDir1 = "/data/user/1/com.google.bar/"; mBarBaseUser0 = new TestData(barPackageName, - barCodeDir + "base.apk", 0, ISA, false, true, barPackageName); + barCodeDir + "base.apk", 0, ISA, true, barPackageName); mBarSecondary1User0 = new TestData(barPackageName, - barDataDir + "sec-1.dex", 0, ISA, false, false, barPackageName); + barDataDir + "sec-1.dex", 0, ISA, false, barPackageName); mBarSecondary2User1 = new TestData(barPackageName, - barDataDir1 + "sec-2.dex", 1, ISA, false, false, barPackageName); + barDataDir1 + "sec-2.dex", 1, ISA, false, barPackageName); } @Test @@ -134,7 +134,9 @@ public class PackageDexUsageTests { public void testRecordSplitPrimarySequence() { // Assert new information. assertTrue(record(mFooBaseUser0)); - // Assert no new information. + assertTrue(record(mFooSplit1User0)); + // Assert no new information if we add again + assertFalse(record(mFooBaseUser0)); assertFalse(record(mFooSplit1User0)); assertPackageDexUsage(mFooBaseUser0); @@ -192,7 +194,7 @@ public class PackageDexUsageTests { for (int i = 1; i <= tooManyFiles; i++) { String fooPackageName = "com.google.foo"; TestData testData = new TestData(fooPackageName, - "/data/user/0/" + fooPackageName + "/sec-" + i + "1.dex", 0, ISA, false, false, + "/data/user/0/" + fooPackageName + "/sec-" + i + "1.dex", 0, ISA, false, fooPackageName); if (i < tooManyFiles) { assertTrue("Adding " + testData.mDexFile, record(testData)); @@ -200,7 +202,11 @@ public class PackageDexUsageTests { } else { assertFalse("Adding " + testData.mDexFile, record(testData)); } - assertPackageDexUsage(mPackageDexUsage, null, null, expectedSecondaries); + assertPackageDexUsage( + mPackageDexUsage, + /* usdeBy=*/ (Set) null, + /* primaryDex= */ null, + expectedSecondaries); } } @@ -345,9 +351,8 @@ public class PackageDexUsageTests { mFooSplit2UsedByOtherApps0.mDexFile, mFooSplit2UsedByOtherApps0.mOwnerUserId, mFooSplit2UsedByOtherApps0.mLoaderIsa, - /*mIsUsedByOtherApps*/false, mFooSplit2UsedByOtherApps0.mPrimaryOrSplit, - mFooSplit2UsedByOtherApps0.mUsedBy); + /*usedBy=*/ null); assertPackageDexUsage(noLongerUsedByOtherApps); } @@ -371,19 +376,19 @@ public class PackageDexUsageTests { assertTrue(record(packageDexUsageRecordUsers, mFooSplit2UsedByOtherApps0, users)); assertTrue(record(packageDexUsageRecordUsers, mFooSplit2UsedByOtherApps0, usersExtra)); - assertTrue(record(packageDexUsageRecordUsers, mFooSecondary1User0, users)); - assertTrue(record(packageDexUsageRecordUsers, mFooSecondary1User0, usersExtra)); + assertTrue(record(packageDexUsageRecordUsers, mFooSecondary2UsedByOtherApps0, users)); + assertTrue(record(packageDexUsageRecordUsers, mFooSecondary2UsedByOtherApps0, usersExtra)); packageDexUsageRecordUsers = writeAndReadBack(packageDexUsageRecordUsers); // Verify that the users were recorded. Set userAll = new HashSet<>(users); userAll.addAll(usersExtra); assertPackageDexUsage(packageDexUsageRecordUsers, userAll, mFooSplit2UsedByOtherApps0, - mFooSecondary1User0); + mFooSecondary2UsedByOtherApps0); } @Test - public void testRecordDexFileUsersNotTheOwningPackage() { + public void testRecordDexFileUsersAndTheOwningPackage() { PackageDexUsage packageDexUsageRecordUsers = new PackageDexUsage(); Set users = new HashSet<>(Arrays.asList( new String[] {mFooSplit2UsedByOtherApps0.mPackageName})); @@ -393,13 +398,13 @@ public class PackageDexUsageTests { assertTrue(record(packageDexUsageRecordUsers, mFooSplit2UsedByOtherApps0, users)); assertTrue(record(packageDexUsageRecordUsers, mFooSplit2UsedByOtherApps0, usersExtra)); - assertTrue(record(packageDexUsageRecordUsers, mFooSecondary1User0, users)); - assertTrue(record(packageDexUsageRecordUsers, mFooSecondary1User0, usersExtra)); - packageDexUsageRecordUsers = writeAndReadBack(packageDexUsageRecordUsers); - // Verify that only the non owning packages were recorded. - assertPackageDexUsage(packageDexUsageRecordUsers, usersExtra, mFooSplit2UsedByOtherApps0, - mFooSecondary1User0); + + Set expectedUsers = new HashSet<>(users); + expectedUsers.addAll(usersExtra); + // Verify that all loading packages were recorded. + assertPackageDexUsage( + packageDexUsageRecordUsers, expectedUsers, mFooSplit2UsedByOtherApps0); } @Test @@ -435,6 +440,85 @@ public class PackageDexUsageTests { assertTrue(variableContext.isVariableClassLoaderContext()); } + @Test + public void testRead() { + String isa = VMRuntime.getInstructionSet(Build.SUPPORTED_ABIS[0]); + // Equivalent to + // record(mFooSplit2UsedByOtherApps0); + // record(mFooSecondary1User0); + // record(mFooSecondary2UsedByOtherApps0); + // record(mBarBaseUser0); + // record(mBarSecondary1User0); + String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__2\n" + + "com.google.foo\n" + + "+/data/app/com.google.foo/split-2.apk\n" + + "@used.by.other.com\n" + + "#/data/user/0/com.google.foo/sec-2.dex\n" + + "0,1," + ISA + "\n" + + "@used.by.other.com\n" + + "PCL[/data/user/0/com.google.foo/sec-2.dex]\n" + + "#/data/user/0/com.google.foo/sec-1.dex\n" + + "0,0," + ISA + "\n" + + "@\n" + + "PCL[/data/user/0/com.google.foo/sec-1.dex]\n" + + "com.google.bar\n" + + "+/data/app/com.google.bar/base.apk\n" + + "@com.google.bar\n" + + "#/data/user/0/com.google.bar/sec-1.dex\n" + + "0,0," + ISA + "\n" + + "@\n" + + "PCL[/data/user/0/com.google.bar/sec-1.dex]"; + + PackageDexUsage packageDexUsage = new PackageDexUsage(); + try { + packageDexUsage.read(new StringReader(content)); + } catch (IOException e) { + fail(); + } + + // After the read we must sync the data to fill the missing information on the code paths. + Map> packageToUsersMap = new HashMap<>(); + Map> packageToCodePaths = new HashMap<>(); + + // Handle foo package. + packageToUsersMap.put( + mFooSplit2UsedByOtherApps0.mPackageName, + new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mOwnerUserId))); + packageToCodePaths.put( + mFooSplit2UsedByOtherApps0.mPackageName, + new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mDexFile, + mFooSplit1User0.mDexFile, mFooBaseUser0.mDexFile))); + // Handle bar package. + packageToUsersMap.put( + mBarBaseUser0.mPackageName, + new HashSet<>(Arrays.asList(mBarBaseUser0.mOwnerUserId))); + packageToCodePaths.put( + mBarBaseUser0.mPackageName, + new HashSet<>(Arrays.asList(mBarBaseUser0.mDexFile))); + // Handle the loading package. + packageToUsersMap.put( + mFooSplit2UsedByOtherApps0.mUsedBy, + new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mOwnerUserId))); + + // Sync the data. + packageDexUsage.syncData(packageToUsersMap, packageToCodePaths); + + // Assert foo code paths. + assertPackageDexUsage( + packageDexUsage, + /*nonDefaultUsers=*/ null, + mFooSplit2UsedByOtherApps0, + mFooSecondary2UsedByOtherApps0, + mFooSecondary1User0); + + // Assert bar code paths. + assertPackageDexUsage( + packageDexUsage, + /*nonDefaultUsers=*/ null, + mBarBaseUser0, + mBarSecondary1User0); + } + @Test public void testUnsupportedClassLoaderDiscardedOnRead() throws Exception { String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__2\n" @@ -470,16 +554,18 @@ public class PackageDexUsageTests { String packageName = primary == null ? secondaries.get(0).mPackageName : primary.mPackageName; - boolean primaryUsedByOtherApps = primary != null && primary.mUsedByOtherApps; + boolean primaryUsedByOtherApps = primary != null && primary.isUsedByOtherApps(); PackageUseInfo pInfo = packageDexUsage.getPackageUseInfo(packageName); // Check package use info assertNotNull(pInfo); if (primary != null) { - assertEquals(primaryUsedByOtherApps, pInfo.isUsedByOtherApps(primary.mDexFile)); if (users != null) { assertEquals(pInfo.getLoadingPackages(primary.mDexFile), users); + } else if (pInfo.getLoadingPackages(primary.mDexFile) != null) { + assertEquals(pInfo.getLoadingPackages(primary.mDexFile), primary.getUsedBy()); } + assertEquals(primaryUsedByOtherApps, pInfo.isUsedByOtherApps(primary.mDexFile)); } Map dexUseInfoMap = pInfo.getDexUseInfoMap(); @@ -489,13 +575,15 @@ public class PackageDexUsageTests { for (TestData testData : secondaries) { DexUseInfo dInfo = dexUseInfoMap.get(testData.mDexFile); assertNotNull(dInfo); - assertEquals(testData.mUsedByOtherApps, dInfo.isUsedByOtherApps()); + if (users != null) { + assertEquals(testData.mDexFile, dInfo.getLoadingPackages(), users); + } else { + assertEquals(testData.mDexFile, dInfo.getLoadingPackages(), testData.getUsedBy()); + } + assertEquals(testData.isUsedByOtherApps(), dInfo.isUsedByOtherApps()); assertEquals(testData.mOwnerUserId, dInfo.getOwnerUserId()); assertEquals(1, dInfo.getLoaderIsas().size()); assertTrue(dInfo.getLoaderIsas().contains(testData.mLoaderIsa)); - if (users != null) { - assertEquals(dInfo.getLoadingPackages(), users); - } assertEquals(testData.mClassLoaderContext, dInfo.getClassLoaderContext()); } @@ -503,7 +591,7 @@ public class PackageDexUsageTests { private boolean record(TestData testData) { return mPackageDexUsage.record(testData.mPackageName, testData.mDexFile, - testData.mOwnerUserId, testData.mLoaderIsa, testData.mUsedByOtherApps, + testData.mOwnerUserId, testData.mLoaderIsa, testData.mPrimaryOrSplit, testData.mUsedBy, testData.mClassLoaderContext); } @@ -511,7 +599,7 @@ public class PackageDexUsageTests { boolean result = true; for (String user : users) { result = result && packageDexUsage.record(testData.mPackageName, testData.mDexFile, - testData.mOwnerUserId, testData.mLoaderIsa, testData.mUsedByOtherApps, + testData.mOwnerUserId, testData.mLoaderIsa, testData.mPrimaryOrSplit, user, testData.mClassLoaderContext); } return result; @@ -540,37 +628,49 @@ public class PackageDexUsageTests { private final String mDexFile; private final int mOwnerUserId; private final String mLoaderIsa; - private final boolean mUsedByOtherApps; private final boolean mPrimaryOrSplit; private final String mUsedBy; private final String mClassLoaderContext; private TestData(String packageName, String dexFile, int ownerUserId, - String loaderIsa, boolean isUsedByOtherApps, boolean primaryOrSplit, String usedBy) { - this(packageName, dexFile, ownerUserId, loaderIsa, isUsedByOtherApps, primaryOrSplit, - usedBy, "DefaultClassLoaderContextFor_" + dexFile); + String loaderIsa, boolean primaryOrSplit, String usedBy) { + this(packageName, dexFile, ownerUserId, loaderIsa, primaryOrSplit, + usedBy, "PCL[" + dexFile + "]"); } private TestData(String packageName, String dexFile, int ownerUserId, - String loaderIsa, boolean isUsedByOtherApps, boolean primaryOrSplit, String usedBy, + String loaderIsa, boolean primaryOrSplit, String usedBy, String classLoaderContext) { mPackageName = packageName; mDexFile = dexFile; mOwnerUserId = ownerUserId; mLoaderIsa = loaderIsa; - mUsedByOtherApps = isUsedByOtherApps; mPrimaryOrSplit = primaryOrSplit; mUsedBy = usedBy; mClassLoaderContext = classLoaderContext; } private TestData updateClassLoaderContext(String newContext) { - return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, mUsedByOtherApps, + return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, mPrimaryOrSplit, mUsedBy, newContext); } private TestData updateUseByOthers(boolean newUsedByOthers) { - return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, newUsedByOthers, + return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, mPrimaryOrSplit, mUsedBy, mClassLoaderContext); } + + private boolean isUsedByOtherApps() { + return mUsedBy != null && !mPackageName.equals(mUsedBy); + } + + private Set getUsedBy() { + Set users = new HashSet<>(); + if ((mUsedBy != null) && (mPrimaryOrSplit || isUsedByOtherApps())) { + // We do not store the loading package for secondary dex files + // which are not used by others. + users.add(mUsedBy); + } + return users; + } } } -- cgit v1.2.3-59-g8ed1b From 7c8fd2978713f7ff943276485f788f2981955631 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Tue, 31 Mar 2020 14:03:01 -0700 Subject: Allow non-user packages to be kept in the use data list By default, we clean up the data from the package-dex-use.list at every boot. The clean up will remove any obsolete or uninstalled packages. Since system server will not show up as a regular app, we need to add a mechanism to ensure that its dex files are not cleaned up. Test: atest DexManagerTests PackageManagerTests Bug: 148774920 Change-Id: I0788163b066af5713b8763f1875338add2d49aa9 --- .../java/com/android/server/pm/dex/DexManager.java | 5 ++- .../com/android/server/pm/dex/PackageDexUsage.java | 32 +++++++++++++--- .../server/pm/dex/PackageDexUsageTests.java | 43 ++++++++++++++++++++-- 3 files changed, 70 insertions(+), 10 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 9f2ec47335f3..e8765ad973f3 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -45,6 +45,7 @@ import dalvik.system.VMRuntime; import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -353,7 +354,9 @@ public class DexManager { try { mPackageDexUsage.read(); - mPackageDexUsage.syncData(packageToUsersMap, packageToCodePaths); + List packagesToKeepDataAbout = new ArrayList<>(); + mPackageDexUsage.syncData( + packageToUsersMap, packageToCodePaths, packagesToKeepDataAbout); } catch (Exception e) { mPackageDexUsage.clear(); Slog.w(TAG, "Exception while loading package dex usage. " 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 1a3c18f61333..7ac09e373d20 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -474,13 +474,19 @@ public class PackageDexUsage extends AbstractStatsBase { * Syncs the existing data with the set of available packages by removing obsolete entries. */ /*package*/ void syncData(Map> packageToUsersMap, - Map> packageToCodePaths) { + Map> packageToCodePaths, + List packagesToKeepDataAbout) { synchronized (mPackageUseInfoMap) { Iterator> pIt = mPackageUseInfoMap.entrySet().iterator(); while (pIt.hasNext()) { Map.Entry pEntry = pIt.next(); String packageName = pEntry.getKey(); + if (packagesToKeepDataAbout.contains(packageName)) { + // This is a package for which we should keep the data even if it's not + // in the list of user packages. + continue; + } PackageUseInfo packageUseInfo = pEntry.getValue(); Set users = packageToUsersMap.get(packageName); if (users == null) { @@ -501,11 +507,27 @@ public class PackageDexUsage extends AbstractStatsBase { // Sync the code paths. Set codePaths = packageToCodePaths.get(packageName); - Iterator>> codeIt = + + Iterator>> recordedIt = packageUseInfo.mPrimaryCodePaths.entrySet().iterator(); - while (codeIt.hasNext()) { - if (!codePaths.contains(codeIt.next().getKey())) { - codeIt.remove(); + while (recordedIt.hasNext()) { + Map.Entry> entry = recordedIt.next(); + String recordedCodePath = entry.getKey(); + if (!codePaths.contains(recordedCodePath)) { + // Clean up a non existing code path. + recordedIt.remove(); + } else { + // Clean up a non existing loading package. + Set recordedLoadingPackages = entry.getValue(); + Iterator recordedLoadingPackagesIt = + recordedLoadingPackages.iterator(); + while (recordedLoadingPackagesIt.hasNext()) { + String recordedLoadingPackage = recordedLoadingPackagesIt.next(); + if (!packagesToKeepDataAbout.contains(recordedLoadingPackage) + && !packageToUsersMap.containsKey(recordedLoadingPackage)) { + recordedLoadingPackagesIt.remove(); + } + } } } 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 08fc87758f57..2829fa7bf83f 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 @@ -282,13 +282,48 @@ public class PackageDexUsageTests { Map> packageToCodePaths = new HashMap<>(); packageToCodePaths.put(mBarBaseUser0.mPackageName, new HashSet<>(Arrays.asList(mBarBaseUser0.mDexFile))); - mPackageDexUsage.syncData(packageToUsersMap, packageToCodePaths); + mPackageDexUsage.syncData(packageToUsersMap, packageToCodePaths, new ArrayList()); // Assert that only user 1 files are there. assertPackageDexUsage(mBarBaseUser0, mBarSecondary2User1); assertNull(mPackageDexUsage.getPackageUseInfo(mFooBaseUser0.mPackageName)); } + @Test + public void testSyncDataKeepPackages() { + PackageDexUsage packageDexUsage = new PackageDexUsage(); + // Write the record we want to keep and which won't be keep by default. + Set fooUsers = new HashSet<>(Arrays.asList( + new String[] {mFooBaseUser0.mPackageName})); + assertTrue(record(packageDexUsage, mFooBaseUser0, fooUsers)); + // Write a record that would be kept by default. + Set barUsers = new HashSet<>(Arrays.asList( + new String[] {"another.package", mFooBaseUser0.mPackageName})); + assertTrue(record(packageDexUsage, mBarBaseUser0, barUsers)); + + // Construct the user packages and their code paths (things that will be + // kept by default during sync). + Map> packageToUsersMap = new HashMap<>(); + packageToUsersMap.put(mBarBaseUser0.mPackageName, + new HashSet<>(Arrays.asList(mBarBaseUser0.mOwnerUserId))); + Map> packageToCodePaths = new HashMap<>(); + packageToCodePaths.put(mBarBaseUser0.mPackageName, + new HashSet<>(Arrays.asList(mBarBaseUser0.mDexFile))); + + // Sync data. + List keepData = new ArrayList(); + keepData.add(mFooBaseUser0.mPackageName); + packageDexUsage.syncData(packageToUsersMap, packageToCodePaths, keepData); + + // Assert that both packages are kept + assertPackageDexUsage(packageDexUsage, fooUsers, mFooBaseUser0); + // "another.package" should not be in the loading packages after sync. + Set expectedBarUsers = new HashSet<>(Arrays.asList( + new String[] {mFooBaseUser0.mPackageName})); + assertPackageDexUsage(packageDexUsage, expectedBarUsers, + mBarBaseUser0.updateUsedBy(mFooBaseUser0.mPackageName)); + } + @Test public void testRemovePackage() { // Record Bar secondaries for two different users. @@ -501,7 +536,7 @@ public class PackageDexUsageTests { new HashSet<>(Arrays.asList(mFooSplit2UsedByOtherApps0.mOwnerUserId))); // Sync the data. - packageDexUsage.syncData(packageToUsersMap, packageToCodePaths); + packageDexUsage.syncData(packageToUsersMap, packageToCodePaths, new ArrayList<>()); // Assert foo code paths. assertPackageDexUsage( @@ -654,9 +689,9 @@ public class PackageDexUsageTests { mPrimaryOrSplit, mUsedBy, newContext); } - private TestData updateUseByOthers(boolean newUsedByOthers) { + private TestData updateUsedBy(String newUsedBy) { return new TestData(mPackageName, mDexFile, mOwnerUserId, mLoaderIsa, - mPrimaryOrSplit, mUsedBy, mClassLoaderContext); + mPrimaryOrSplit, newUsedBy, mClassLoaderContext); } private boolean isUsedByOtherApps() { -- cgit v1.2.3-59-g8ed1b From c2deeefef2abff2ce355d012a0bd2b1cd8258d18 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Tue, 31 Mar 2020 19:51:34 -0700 Subject: Ensure that loading packages can be extended Collections.emptySet() creates an immutable collection which will prevent us to add more packages to the loading packages set. Instead of using the emptySet, create a new set everytime, even if it's empty. Test: PackageDexUseTest Bug: 148774920 Change-Id: If274d0380d494d31768da24c10153810ef8bf513 --- .../java/com/android/server/pm/dex/PackageDexUsage.java | 9 +++------ .../com/android/server/pm/dex/PackageDexUsageTests.java | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) 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 7ac09e373d20..10760f52a02b 100644 --- a/services/core/java/com/android/server/pm/dex/PackageDexUsage.java +++ b/services/core/java/com/android/server/pm/dex/PackageDexUsage.java @@ -446,15 +446,12 @@ public class PackageDexUsage extends AbstractStatsBase { if (line == null) { throw new IllegalStateException("Could not find the loadingPackages line."); } - // We expect that most of the times the list of loading packages will be empty. - if (line.length() == LOADING_PACKAGE_CHAR.length()) { - return Collections.emptySet(); - } else { - Set result = new HashSet<>(); + Set result = new HashSet<>(); + if (line.length() != LOADING_PACKAGE_CHAR.length()) { Collections.addAll(result, line.substring(LOADING_PACKAGE_CHAR.length()).split(SPLIT_CHAR)); - return result; } + return result; } /** 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 2829fa7bf83f..adf4551e79a8 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 @@ -575,6 +575,22 @@ public class PackageDexUsageTests { assertPackageDexUsage(mBarBaseUser0); } + @Test + public void testEnsureLoadingPackagesCanBeExtended() { + String isa = VMRuntime.getInstructionSet(Build.SUPPORTED_ABIS[0]); + String content = "PACKAGE_MANAGER__PACKAGE_DEX_USAGE__2\n" + + "com.google.foo\n" + + "+/data/app/com.google.foo/split-2.apk\n" + + "@\n"; + PackageDexUsage packageDexUsage = new PackageDexUsage(); + try { + packageDexUsage.read(new StringReader(content)); + } catch (IOException e) { + fail(); + } + record(packageDexUsage, mFooSplit2UsedByOtherApps0, mFooSplit2UsedByOtherApps0.getUsedBy()); + } + private void assertPackageDexUsage(TestData primary, TestData... secondaries) { assertPackageDexUsage(mPackageDexUsage, null, primary, secondaries); } -- cgit v1.2.3-59-g8ed1b