From fbdfba51b3905d8ab8a8d100f39ac6b33462adb2 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Thu, 3 Nov 2022 00:07:50 -0700 Subject: [PATCH 1/2] Make ApkAssets::IsUpToDate() @CriticalNative This function keeps being called hundreds times per second Bug: 237583012 Test: Build + boot + UTs Change-Id: Id6e0e49d561e0c81f6960a5fd77b2076892bd339 --- core/java/android/content/res/ApkAssets.java | 4 +- core/jni/android_content_res_ApkAssets.cpp | 42 +++++++++++--------- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/core/java/android/content/res/ApkAssets.java b/core/java/android/content/res/ApkAssets.java index 7a5ac8ede4a42..143c00dd4d81e 100644 --- a/core/java/android/content/res/ApkAssets.java +++ b/core/java/android/content/res/ApkAssets.java @@ -26,6 +26,8 @@ import android.text.TextUtils; import com.android.internal.annotations.GuardedBy; +import dalvik.annotation.optimization.CriticalNative; + import java.io.FileDescriptor; import java.io.IOException; import java.io.PrintWriter; @@ -459,7 +461,7 @@ public final class ApkAssets { private static native @NonNull String nativeGetAssetPath(long ptr); private static native @NonNull String nativeGetDebugName(long ptr); private static native long nativeGetStringBlock(long ptr); - private static native boolean nativeIsUpToDate(long ptr); + @CriticalNative private static native boolean nativeIsUpToDate(long ptr); private static native long nativeOpenXml(long ptr, @NonNull String fileName) throws IOException; private static native @Nullable OverlayableInfo nativeGetOverlayableInfo(long ptr, String overlayableName) throws IOException; diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp index 5a78412ac2772..29560dce1cd86 100644 --- a/core/jni/android_content_res_ApkAssets.cpp +++ b/core/jni/android_content_res_ApkAssets.cpp @@ -402,7 +402,7 @@ static jlong NativeGetStringBlock(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr) return reinterpret_cast(apk_assets->GetLoadedArsc()->GetStringPool()); } -static jboolean NativeIsUpToDate(JNIEnv* /*env*/, jclass /*clazz*/, jlong ptr) { +static jboolean NativeIsUpToDate(jlong ptr) { auto scoped_apk_assets = ScopedLock(ApkAssetsFromLong(ptr)); auto apk_assets = scoped_apk_assets->get(); return apk_assets->IsUpToDate() ? JNI_TRUE : JNI_FALSE; @@ -500,24 +500,28 @@ static jboolean NativeDefinesOverlayable(JNIEnv* env, jclass /*clazz*/, jlong pt // JNI registration. static const JNINativeMethod gApkAssetsMethods[] = { - {"nativeLoad", "(ILjava/lang/String;ILandroid/content/res/loader/AssetsProvider;)J", - (void*)NativeLoad}, - {"nativeLoadEmpty", "(ILandroid/content/res/loader/AssetsProvider;)J", (void*)NativeLoadEmpty}, - {"nativeLoadFd", - "(ILjava/io/FileDescriptor;Ljava/lang/String;ILandroid/content/res/loader/AssetsProvider;)J", - (void*)NativeLoadFromFd}, - {"nativeLoadFdOffsets", - "(ILjava/io/FileDescriptor;Ljava/lang/String;JJILandroid/content/res/loader/AssetsProvider;)J", - (void*)NativeLoadFromFdOffset}, - {"nativeDestroy", "(J)V", (void*)NativeDestroy}, - {"nativeGetAssetPath", "(J)Ljava/lang/String;", (void*)NativeGetAssetPath}, - {"nativeGetDebugName", "(J)Ljava/lang/String;", (void*)NativeGetDebugName}, - {"nativeGetStringBlock", "(J)J", (void*)NativeGetStringBlock}, - {"nativeIsUpToDate", "(J)Z", (void*)NativeIsUpToDate}, - {"nativeOpenXml", "(JLjava/lang/String;)J", (void*)NativeOpenXml}, - {"nativeGetOverlayableInfo", "(JLjava/lang/String;)Landroid/content/om/OverlayableInfo;", - (void*)NativeGetOverlayableInfo}, - {"nativeDefinesOverlayable", "(J)Z", (void*)NativeDefinesOverlayable}, + {"nativeLoad", "(ILjava/lang/String;ILandroid/content/res/loader/AssetsProvider;)J", + (void*)NativeLoad}, + {"nativeLoadEmpty", "(ILandroid/content/res/loader/AssetsProvider;)J", + (void*)NativeLoadEmpty}, + {"nativeLoadFd", + "(ILjava/io/FileDescriptor;Ljava/lang/String;ILandroid/content/res/loader/" + "AssetsProvider;)J", + (void*)NativeLoadFromFd}, + {"nativeLoadFdOffsets", + "(ILjava/io/FileDescriptor;Ljava/lang/String;JJILandroid/content/res/loader/" + "AssetsProvider;)J", + (void*)NativeLoadFromFdOffset}, + {"nativeDestroy", "(J)V", (void*)NativeDestroy}, + {"nativeGetAssetPath", "(J)Ljava/lang/String;", (void*)NativeGetAssetPath}, + {"nativeGetDebugName", "(J)Ljava/lang/String;", (void*)NativeGetDebugName}, + {"nativeGetStringBlock", "(J)J", (void*)NativeGetStringBlock}, + // @CriticalNative + {"nativeIsUpToDate", "(J)Z", (void*)NativeIsUpToDate}, + {"nativeOpenXml", "(JLjava/lang/String;)J", (void*)NativeOpenXml}, + {"nativeGetOverlayableInfo", "(JLjava/lang/String;)Landroid/content/om/OverlayableInfo;", + (void*)NativeGetOverlayableInfo}, + {"nativeDefinesOverlayable", "(J)Z", (void*)NativeDefinesOverlayable}, }; int register_android_content_res_ApkAssets(JNIEnv* env) { From 59dab3ba294e252e85a5bf7fc0926a448111ad19 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Thu, 3 Nov 2022 00:08:49 -0700 Subject: [PATCH 2/2] Optimize filter/alias rebuilding in AssetManager2 + make the default member of ByteBucketArray static Bug: 237583012 Test: build + UTs Change-Id: I94af279b7e496488b23bd12f071f5fcc7738c194 --- libs/androidfw/AssetManager2.cpp | 53 +++++++++++-------- libs/androidfw/ResourceTypes.cpp | 2 +- .../include/androidfw/ByteBucketArray.h | 10 +++- .../include/androidfw/ResourceTypes.h | 16 +++--- 4 files changed, 49 insertions(+), 32 deletions(-) diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 27fb94408966f..61e842a359cec 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include "android-base/logging.h" #include "android-base/stringprintf.h" @@ -207,23 +208,30 @@ void AssetManager2::BuildDynamicRefTable() { } // Now assign the runtime IDs so that we have a build-time to runtime ID map. - const auto package_groups_end = package_groups_.end(); - for (auto iter = package_groups_.begin(); iter != package_groups_end; ++iter) { - const std::string& package_name = iter->packages_[0].loaded_package_->GetPackageName(); - for (auto iter2 = package_groups_.begin(); iter2 != package_groups_end; ++iter2) { - iter2->dynamic_ref_table->addMapping(String16(package_name.c_str(), package_name.size()), - iter->dynamic_ref_table->mAssignedPackageId); - - // Add the alias resources to the dynamic reference table of every package group. Since - // staging aliases can only be defined by the framework package (which is not a shared - // library), the compile-time package id of the framework is the same across all packages - // that compile against the framework. - for (const auto& package : iter->packages_) { - for (const auto& entry : package.loaded_package_->GetAliasResourceIdMap()) { - iter2->dynamic_ref_table->addAlias(entry.first, entry.second); - } - } + DynamicRefTable::AliasMap aliases; + for (const auto& group : package_groups_) { + const std::string& package_name = group.packages_[0].loaded_package_->GetPackageName(); + const auto name_16 = String16(package_name.c_str(), package_name.size()); + for (auto&& inner_group : package_groups_) { + inner_group.dynamic_ref_table->addMapping(name_16, + group.dynamic_ref_table->mAssignedPackageId); } + + for (const auto& package : group.packages_) { + const auto& package_aliases = package.loaded_package_->GetAliasResourceIdMap(); + aliases.insert(package_aliases.begin(), package_aliases.end()); + } + } + + if (!aliases.empty()) { + // Add the alias resources to the dynamic reference table of every package group. Since + // staging aliases can only be defined by the framework package (which is not a shared + // library), the compile-time package id of the framework is the same across all packages + // that compile against the framework. + for (auto& group : std::span(package_groups_.data(), package_groups_.size() - 1)) { + group.dynamic_ref_table->setAliases(aliases); + } + package_groups_.back().dynamic_ref_table->setAliases(std::move(aliases)); } } @@ -1347,18 +1355,17 @@ base::expected AssetManager2::GetResourceId( void AssetManager2::RebuildFilterList() { for (PackageGroup& group : package_groups_) { for (ConfiguredPackage& impl : group.packages_) { - // Destroy it. - impl.filtered_configs_.~ByteBucketArray(); - - // Re-create it. - new (&impl.filtered_configs_) ByteBucketArray(); + impl.filtered_configs_.clear(); // Create the filters here. impl.loaded_package_->ForEachTypeSpec([&](const TypeSpec& type_spec, uint8_t type_id) { - FilteredConfigGroup& group = impl.filtered_configs_.editItemAt(type_id - 1); + FilteredConfigGroup* group = nullptr; for (const auto& type_entry : type_spec.type_entries) { if (type_entry.config.match(configuration_)) { - group.type_entries.push_back(&type_entry); + if (!group) { + group = &impl.filtered_configs_.editItemAt(type_id - 1); + } + group->type_entries.push_back(&type_entry); } } }); diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp index 267190a54195c..035ed4f394554 100644 --- a/libs/androidfw/ResourceTypes.cpp +++ b/libs/androidfw/ResourceTypes.cpp @@ -7074,7 +7074,7 @@ void DynamicRefTable::addMapping(uint8_t buildPackageId, uint8_t runtimePackageI } void DynamicRefTable::addAlias(uint32_t stagedId, uint32_t finalizedId) { - mAliasId[stagedId] = finalizedId; + mAliasId[stagedId] = finalizedId; } status_t DynamicRefTable::lookupResourceId(uint32_t* resId) const { diff --git a/libs/androidfw/include/androidfw/ByteBucketArray.h b/libs/androidfw/include/androidfw/ByteBucketArray.h index 949c9445b3e8b..05a2c4db8fe96 100644 --- a/libs/androidfw/include/androidfw/ByteBucketArray.h +++ b/libs/androidfw/include/androidfw/ByteBucketArray.h @@ -31,9 +31,15 @@ namespace android { template class ByteBucketArray { public: - ByteBucketArray() : default_() { memset(buckets_, 0, sizeof(buckets_)); } + ByteBucketArray() { + memset(buckets_, 0, sizeof(buckets_)); + } ~ByteBucketArray() { + clear(); + } + + void clear() { for (size_t i = 0; i < kNumBuckets; i++) { if (buckets_[i] != NULL) { delete[] buckets_[i]; @@ -84,7 +90,7 @@ class ByteBucketArray { enum { kNumBuckets = 16, kBucketSize = 16 }; T* buckets_[kNumBuckets]; - T default_; + static inline const T default_ = {}; }; } // namespace android diff --git a/libs/androidfw/include/androidfw/ResourceTypes.h b/libs/androidfw/include/androidfw/ResourceTypes.h index b2b95b72e0e0d..d98e97ad32f08 100644 --- a/libs/androidfw/include/androidfw/ResourceTypes.h +++ b/libs/androidfw/include/androidfw/ResourceTypes.h @@ -1882,6 +1882,10 @@ public: void addMapping(uint8_t buildPackageId, uint8_t runtimePackageId); + using AliasMap = std::map; + void setAliases(AliasMap aliases) { + mAliasId = std::move(aliases); + } void addAlias(uint32_t stagedId, uint32_t finalizedId); // Returns whether or not the value must be looked up. @@ -1896,12 +1900,12 @@ public: return mEntries; } -private: - uint8_t mAssignedPackageId; - uint8_t mLookupTable[256]; - KeyedVector mEntries; - bool mAppAsLib; - std::map mAliasId; + private: + uint8_t mLookupTable[256]; + uint8_t mAssignedPackageId; + bool mAppAsLib; + KeyedVector mEntries; + AliasMap mAliasId; }; bool U16StringToInt(const char16_t* s, size_t len, Res_value* outValue);