[res] Correct, optimize adding shared lib assets
When we add shared library assets to all ResourceImpl objects
there are several important details:
1. ApkAssets array contains both file-based and loader-based
assets, and they have to be partitioned in this order,
otherwise loaders won't be able to overlay all file assets
2. When adding all shared library assets they also may contain
frros, and those need to be loaded as overlays explicitly
3. No need to repeatedly add each asset path as it involves the
same steps of checking for duplicates and rebuilding the
native structures
4. Given it's the constructor and it will force-set the
configuration later, adding assets can be done in 'preset'
mode without calculating anything config-related.
Bug: 345562237
Test: build, atest ResourcesManagerTest, simpleperf FRRO changes
Change-Id: Ia000549bbec06d5b8f649c7bff636c4f2a1dac22
diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java
index 273e40a..c0c1c31 100644
--- a/core/java/android/content/res/AssetManager.java
+++ b/core/java/android/content/res/AssetManager.java
@@ -32,6 +32,7 @@
import android.content.res.loader.ResourcesLoader;
import android.os.Build;
import android.os.ParcelFileDescriptor;
+import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
import android.util.SparseArray;
@@ -448,7 +449,7 @@
@Deprecated
@UnsupportedAppUsage
public int addAssetPath(String path) {
- return addAssetPathInternal(path, false /*overlay*/, false /*appAsLib*/);
+ return addAssetPathInternal(List.of(path), false, false, false);
}
/**
@@ -458,7 +459,7 @@
@Deprecated
@UnsupportedAppUsage
public int addAssetPathAsSharedLibrary(String path) {
- return addAssetPathInternal(path, false /*overlay*/, true /*appAsLib*/);
+ return addAssetPathInternal(List.of(path), false, true, false);
}
/**
@@ -468,35 +469,103 @@
@Deprecated
@UnsupportedAppUsage
public int addOverlayPath(String path) {
- return addAssetPathInternal(path, true /*overlay*/, false /*appAsLib*/);
+ return addAssetPathInternal(List.of(path), true, false, false);
}
/**
* @hide
*/
- public void addSharedLibraryPaths(@NonNull String[] paths) {
- final int length = paths.length;
- for (int i = 0; i < length; i++) {
- addAssetPathInternal(paths[i], false, true);
+ public void addSharedLibraryPaths(@NonNull List<String> paths) {
+ addAssetPathInternal(paths, false, true, true);
+ }
+
+ private int addAssetPathInternal(List<String> paths, boolean overlay, boolean appAsLib,
+ boolean presetAssets) {
+ Objects.requireNonNull(paths, "paths");
+ if (paths.isEmpty()) {
+ return 0;
+ }
+
+ synchronized (this) {
+ ensureOpenLocked();
+
+ // See if we already have some of the paths loaded.
+ final int originalAssetsCount = mApkAssets.length;
+
+ // Getting an assets' path is a relatively expensive operation, cache them.
+ final ArrayMap<String, Integer> assetPaths = new ArrayMap<>(originalAssetsCount);
+ for (int i = 0; i < originalAssetsCount; i++) {
+ assetPaths.put(mApkAssets[i].getAssetPath(), i);
+ }
+
+ final ArrayList<String> newPaths = new ArrayList<>(paths.size());
+ int lastFoundIndex = -1;
+ for (int i = 0, pathsSize = paths.size(); i < pathsSize; i++) {
+ final var path = paths.get(i);
+ final int index = assetPaths.getOrDefault(path, -1);
+ if (index < 0) {
+ newPaths.add(path);
+ } else {
+ lastFoundIndex = index;
+ }
+ }
+ if (newPaths.isEmpty()) {
+ return lastFoundIndex + 1;
+ }
+
+ final var newAssets = loadAssets(newPaths, overlay, appAsLib);
+ if (newAssets.isEmpty()) {
+ return 0;
+ }
+ mApkAssets = makeNewAssetsArrayLocked(newAssets);
+ nativeSetApkAssets(mObject, mApkAssets, true, presetAssets);
+ invalidateCachesLocked(-1);
+ return originalAssetsCount + 1;
}
}
- private int addAssetPathInternal(String path, boolean overlay, boolean appAsLib) {
- Objects.requireNonNull(path, "path");
- synchronized (this) {
- ensureOpenLocked();
- final int count = mApkAssets.length;
-
- // See if we already have it loaded.
- for (int i = 0; i < count; i++) {
- if (mApkAssets[i].getAssetPath().equals(path)) {
- return i + 1;
- }
+ /**
+ * Insert the new assets preserving the correct order: all non-loader assets go before all
+ * of the loader assets.
+ */
+ @GuardedBy("this")
+ private @NonNull ApkAssets[] makeNewAssetsArrayLocked(
+ @NonNull ArrayList<ApkAssets> newNonLoaderAssets) {
+ final int originalAssetsCount = mApkAssets.length;
+ int firstLoaderIndex = originalAssetsCount;
+ for (int i = 0; i < originalAssetsCount; i++) {
+ if (mApkAssets[i].isForLoader()) {
+ firstLoaderIndex = i;
+ break;
}
+ }
+ final int newAssetsSize = newNonLoaderAssets.size();
+ final var newAssetsArray = new ApkAssets[originalAssetsCount + newAssetsSize];
+ if (firstLoaderIndex > 0) {
+ // This should always be true, but who knows...
+ System.arraycopy(mApkAssets, 0, newAssetsArray, 0, firstLoaderIndex);
+ }
+ for (int i = 0; i < newAssetsSize; i++) {
+ newAssetsArray[firstLoaderIndex + i] = newNonLoaderAssets.get(i);
+ }
+ if (originalAssetsCount > firstLoaderIndex) {
+ System.arraycopy(
+ mApkAssets, firstLoaderIndex,
+ newAssetsArray, firstLoaderIndex + newAssetsSize,
+ originalAssetsCount - firstLoaderIndex);
+ }
+ return newAssetsArray;
+ }
- final ApkAssets assets;
+ private static @NonNull ArrayList<ApkAssets> loadAssets(@NonNull ArrayList<String> paths,
+ boolean overlay, boolean appAsLib) {
+ final int pathsSize = paths.size();
+ final var loadedAssets = new ArrayList<ApkAssets>(pathsSize);
+ for (int i = 0; i < pathsSize; i++) {
+ final var path = paths.get(i);
try {
- if (overlay) {
+ final ApkAssets assets;
+ if (overlay || path.endsWith(".frro")) {
// TODO(b/70343104): This hardcoded path will be removed once
// addAssetPathInternal is deleted.
final String idmapPath = "/data/resource-cache/"
@@ -507,16 +576,12 @@
assets = ApkAssets.loadFromPath(path,
appAsLib ? ApkAssets.PROPERTY_DYNAMIC : 0);
}
+ loadedAssets.add(assets);
} catch (IOException e) {
- return 0;
+ Log.w(TAG, "Failed to load asset, path = " + path, e);
}
-
- mApkAssets = Arrays.copyOf(mApkAssets, count + 1);
- mApkAssets[count] = assets;
- nativeSetApkAssets(mObject, mApkAssets, true, false);
- invalidateCachesLocked(-1);
- return count + 1;
}
+ return loadedAssets;
}
/** @hide */
diff --git a/core/java/android/content/res/ResourcesImpl.java b/core/java/android/content/res/ResourcesImpl.java
index 31cacb7..9f8b974 100644
--- a/core/java/android/content/res/ResourcesImpl.java
+++ b/core/java/android/content/res/ResourcesImpl.java
@@ -49,6 +49,7 @@
import android.os.ParcelFileDescriptor;
import android.os.Trace;
import android.util.ArrayMap;
+import android.util.ArraySet;
import android.util.AttributeSet;
import android.util.DisplayMetrics;
import android.util.Log;
@@ -71,6 +72,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintWriter;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Locale;
@@ -205,13 +207,21 @@
@Nullable Configuration config, @NonNull DisplayAdjustments displayAdjustments) {
mAssets = assets;
if (Flags.registerResourcePaths()) {
- ArrayMap<String, SharedLibraryAssets> sharedLibMap =
+ final ArraySet<String> uniquePaths = new ArraySet<>();
+ final ArrayList<String> orderedPaths = new ArrayList<>();
+ final ArrayMap<String, SharedLibraryAssets> sharedLibMap =
ResourcesManager.getInstance().getSharedLibAssetsMap();
final int size = sharedLibMap.size();
for (int i = 0; i < size; i++) {
- assets.addSharedLibraryPaths(sharedLibMap.valueAt(i).getAllAssetPaths());
+ final var paths = sharedLibMap.valueAt(i).getAllAssetPaths();
+ for (int j = 0; j < paths.length; j++) {
+ if (uniquePaths.add(paths[j])) {
+ orderedPaths.add(paths[j]);
+ }
+ }
}
- mSharedLibCount = sharedLibMap.size();
+ assets.addSharedLibraryPaths(orderedPaths);
+ mSharedLibCount = size;
}
mMetrics.setToDefaults();
mDisplayAdjustments = displayAdjustments;