From 657f3c6bbfb7b0a7782562c5de5fd1aecc1346b3 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Wed, 10 May 2023 13:16:23 -0700 Subject: [res] Speed up AssetManager pointer locking AssetManager needs to lock (promote()) apk asset weak pointers to use them in pretty much any operation, and often mulitple times for the same object. This CL introduces a concept of an 'operation' in AssetManager, and adds a cache for the locked assets that are retained until the end of that operation. This way we only need to lock each assets object exactly once, losing pretty much no performance. Benchmarks all returned to the pre-memory-safe state values. Bug: 280951693 Bug: 197260547 Bug: 276922628 Test: UTs + native benchmarks + java benchmarks + boot (cherry-picked from https://googleplex-android-review.git.corp.google.com/c/platform/frameworks/base/+/23146856) Merged-In: I26ef88df4f6267b5e8b89f1588f2382db74030c0 Change-Id: I26ef88df4f6267b5e8b89f1588f2382db74030c0 --- libs/androidfw/AssetManager2.cpp | 137 ++++++++++++++++------- libs/androidfw/include/androidfw/AssetManager2.h | 31 ++++- libs/androidfw/tests/AssetManager2_test.cpp | 32 +++++- libs/androidfw/tests/Idmap_test.cpp | 4 +- 4 files changed, 151 insertions(+), 53 deletions(-) (limited to 'libs/androidfw') diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp index 3a7c91ef5b08..d33b592bddc6 100644 --- a/libs/androidfw/AssetManager2.cpp +++ b/libs/androidfw/AssetManager2.cpp @@ -112,7 +112,15 @@ bool AssetManager2::SetApkAssets(std::initializer_list apk_assets, } void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { - apk_assets_.assign(apk_assets.begin(), apk_assets.end()); + auto op = StartOperation(); + + apk_assets_.resize(apk_assets.size()); + for (size_t i = 0; i != apk_assets.size(); ++i) { + apk_assets_[i].first = apk_assets[i]; + // Let's populate the locked assets right away as we're going to need them here later. + apk_assets_[i].second = apk_assets[i]; + } + package_groups_.clear(); package_ids_.fill(0xff); @@ -124,7 +132,7 @@ void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { // Overlay resources are not directly referenced by an application so their resource ids // can change throughout the application's lifetime. Assign overlay package ids last. std::vector sorted_apk_assets; - sorted_apk_assets.reserve(apk_assets_.size()); + sorted_apk_assets.reserve(apk_assets.size()); for (auto& asset : apk_assets) { sorted_apk_assets.push_back(asset.get()); } @@ -250,9 +258,10 @@ void AssetManager2::BuildDynamicRefTable(ApkAssetsList apk_assets) { void AssetManager2::DumpToLog() const { LOG(INFO) << base::StringPrintf("AssetManager2(this=%p)", this); + auto op = StartOperation(); std::string list; - for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); + for (size_t i = 0; i < apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); base::StringAppendF(&list, "%s,", assets ? assets->GetDebugName().c_str() : "nullptr"); } LOG(INFO) << "ApkAssets: " << list; @@ -290,7 +299,8 @@ const ResStringPool* AssetManager2::GetStringPoolForCookie(ApkAssetsCookie cooki if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return nullptr; } - auto assets = apk_assets_[cookie].promote(); + auto op = StartOperation(); + const auto& assets = GetApkAssets(cookie); return assets ? assets->GetLoadedArsc()->GetStringPool() : nullptr; } @@ -341,9 +351,10 @@ const std::unordered_map* bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, std::string* out) const { + auto op = StartOperation(); uint8_t package_id = 0U; - for (const auto& apk_assets : apk_assets_) { - auto assets = apk_assets.promote(); + for (size_t i = 0; i != apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); if (!assets) { continue; } @@ -400,10 +411,14 @@ bool AssetManager2::GetOverlayablesToString(android::StringPiece package_name, } bool AssetManager2::ContainsAllocatedTable() const { - return std::find_if(apk_assets_.begin(), apk_assets_.end(), [](auto&& assets_weak) { - auto assets = assets_weak.promote(); - return assets && assets->IsTableAllocated(); - }) != apk_assets_.end(); + auto op = StartOperation(); + for (size_t i = 0; i != apk_assets_.size(); ++i) { + const auto& assets = GetApkAssets(i); + if (assets && assets->IsTableAllocated()) { + return true; + } + } + return false; } void AssetManager2::SetConfiguration(const ResTable_config& configuration) { @@ -428,8 +443,9 @@ std::set AssetManager2::GetNonSystemOverlays() cons } if (!found_system_package) { + auto op = StartOperation(); for (const ConfiguredOverlay& overlay : package_group.overlays_) { - if (auto asset = apk_assets_[overlay.cookie].promote()) { + if (const auto& asset = GetApkAssets(overlay.cookie)) { non_system_overlays.insert(std::move(asset)); } } @@ -442,6 +458,8 @@ std::set AssetManager2::GetNonSystemOverlays() cons base::expected, IOError> AssetManager2::GetResourceConfigurations( bool exclude_system, bool exclude_mipmap) const { ATRACE_NAME("AssetManager::GetResourceConfigurations"); + auto op = StartOperation(); + const auto non_system_overlays = exclude_system ? GetNonSystemOverlays() : std::set(); @@ -455,7 +473,7 @@ base::expected, IOError> AssetManager2::GetResourceCon } if (!non_system_overlays.empty()) { // Exclude overlays that target only system resources. - auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + const auto& apk_assets = GetApkAssets(package_group.cookies_[i]); if (apk_assets && apk_assets->IsOverlay() && non_system_overlays.find(apk_assets) == non_system_overlays.end()) { continue; @@ -475,6 +493,8 @@ base::expected, IOError> AssetManager2::GetResourceCon std::set AssetManager2::GetResourceLocales(bool exclude_system, bool merge_equivalent_languages) const { ATRACE_NAME("AssetManager::GetResourceLocales"); + auto op = StartOperation(); + std::set locales; const auto non_system_overlays = exclude_system ? GetNonSystemOverlays() : std::set(); @@ -488,7 +508,7 @@ std::set AssetManager2::GetResourceLocales(bool exclude_system, } if (!non_system_overlays.empty()) { // Exclude overlays that target only system resources. - auto apk_assets = apk_assets_[package_group.cookies_[i]].promote(); + const auto& apk_assets = GetApkAssets(package_group.cookies_[i]); if (apk_assets && apk_assets->IsOverlay() && non_system_overlays.find(apk_assets) == non_system_overlays.end()) { continue; @@ -516,13 +536,14 @@ std::unique_ptr AssetManager2::Open(const std::string& filename, ApkAsset std::unique_ptr AssetManager2::OpenDir(const std::string& dirname) const { ATRACE_NAME("AssetManager::OpenDir"); + auto op = StartOperation(); std::string full_path = "assets/" + dirname; auto files = util::make_unique>(); // Start from the back. - for (auto iter = apk_assets_.rbegin(); iter != apk_assets_.rend(); ++iter) { - auto apk_assets = iter->promote(); + for (size_t i = apk_assets_.size(); i > 0; --i) { + const auto& apk_assets = GetApkAssets(i - 1); if (!apk_assets || apk_assets->IsOverlay()) { continue; } @@ -551,8 +572,9 @@ std::unique_ptr AssetManager2::OpenDir(const std::string& dirname) con std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, Asset::AccessMode mode, ApkAssetsCookie* out_cookie) const { - for (int32_t i = apk_assets_.size() - 1; i >= 0; i--) { - const auto assets = apk_assets_[i].promote(); + auto op = StartOperation(); + for (size_t i = apk_assets_.size(); i > 0; i--) { + const auto& assets = GetApkAssets(i - 1); // Prevent RRO from modifying assets and other entries accessed by file // path. Explicitly asking for a path in a given package (denoted by a // cookie) is still OK. @@ -581,7 +603,8 @@ std::unique_ptr AssetManager2::OpenNonAsset(const std::string& filename, if (cookie < 0 || static_cast(cookie) >= apk_assets_.size()) { return {}; } - auto assets = apk_assets_[cookie].promote(); + auto op = StartOperation(); + const auto& assets = GetApkAssets(cookie); return assets ? assets->GetAssetsProvider()->Open(filename, mode) : nullptr; } @@ -595,6 +618,8 @@ base::expected AssetManager2::FindEntry( last_resolution_.resid = resid; } + auto op = StartOperation(); + // Might use this if density_override != 0. ResTable_config density_override_config; @@ -631,7 +656,7 @@ base::expected AssetManager2::FindEntry( bool overlaid = false; if (!stop_at_first_match && !ignore_configuration) { - auto assets = apk_assets_[result->cookie].promote(); + const auto& assets = GetApkAssets(result->cookie); if (!assets) { ALOGE("Found expired ApkAssets #%d for resource ID 0x%08x.", result->cookie, resid); return base::unexpected(std::nullopt); @@ -873,13 +898,7 @@ base::expected AssetManager2::FindEntryInternal( } void AssetManager2::ResetResourceResolution() const { - last_resolution_.cookie = kInvalidCookie; - last_resolution_.resid = 0; - last_resolution_.steps.clear(); - last_resolution_.type_string_ref = StringPoolRef(); - last_resolution_.entry_string_ref = StringPoolRef(); - last_resolution_.best_config_name.clear(); - last_resolution_.best_package_name.clear(); + last_resolution_ = Resolution{}; } void AssetManager2::SetResourceResolutionLoggingEnabled(bool enabled) { @@ -901,8 +920,10 @@ std::string AssetManager2::GetLastResourceResolution() const { return {}; } + auto op = StartOperation(); + const uint32_t resid = last_resolution_.resid; - auto assets = apk_assets_[cookie].promote(); + const auto& assets = GetApkAssets(cookie); const auto package = assets ? assets->GetLoadedArsc()->GetPackageById(get_package_id(resid)) : nullptr; @@ -934,7 +955,7 @@ std::string AssetManager2::GetLastResourceResolution() const { if (prefix == kStepStrings.end()) { continue; } - auto assets = apk_assets_[step.cookie].promote(); + const auto& assets = GetApkAssets(step.cookie); log_stream << "\n\t" << prefix->second << ": " << (assets ? assets->GetDebugName() : "") << " #" << step.cookie; if (!step.config_name.isEmpty()) { @@ -1466,6 +1487,37 @@ void AssetManager2::ForEachPackage(base::function_ref 0) << "Must have an operation running"; + + if (cookie < 0 || cookie >= apk_assets_.size()) { + static const ApkAssetsPtr empty{}; + return empty; + } + auto& [wptr, res] = apk_assets_[cookie]; + if (!res) { + res = wptr.promote(); + } + return res; +} + Theme::Theme(AssetManager2* asset_manager) : asset_manager_(asset_manager) { } @@ -1598,22 +1650,16 @@ base::expected Theme::SetTo(const Theme& source) { using SourceToDestinationRuntimePackageMap = std::unordered_map; std::unordered_map src_asset_cookie_id_map; - // Determine which ApkAssets are loaded in both theme AssetManagers. - const auto& src_assets = source.asset_manager_->GetApkAssets(); - const auto& dest_assets = asset_manager_->GetApkAssets(); - std::vector promoted_src_assets; - promoted_src_assets.reserve(src_assets.size()); - for (const auto& src_asset : src_assets) { - promoted_src_assets.emplace_back(src_asset.promote()); - } + auto op_src = source.asset_manager_->StartOperation(); + auto op_dst = asset_manager_->StartOperation(); - for (size_t j = 0; j < dest_assets.size(); j++) { - auto dest_asset = dest_assets[j].promote(); - if (!dest_asset) { + for (size_t i = 0; i < source.asset_manager_->GetApkAssetsCount(); i++) { + const auto& src_asset = source.asset_manager_->GetApkAssets(i); + if (!src_asset) { continue; } - for (size_t i = 0; i < promoted_src_assets.size(); i++) { - const auto& src_asset = promoted_src_assets[i]; + for (int j = 0; j < asset_manager_->GetApkAssetsCount(); j++) { + const auto& dest_asset = asset_manager_->GetApkAssets(j); if (src_asset != dest_asset) { // ResourcesManager caches and reuses ApkAssets when the same apk must be present in // multiple AssetManagers. Two ApkAssets point to the same version of the same resources @@ -1739,4 +1785,11 @@ void Theme::Dump() const { } } +AssetManager2::ScopedOperation::ScopedOperation(const AssetManager2& am) : am_(am) { +} + +AssetManager2::ScopedOperation::~ScopedOperation() { + am_.FinishOperation(); +} + } // namespace android diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h index e276cd211ee7..fc391bc2ce67 100644 --- a/libs/androidfw/include/androidfw/AssetManager2.h +++ b/libs/androidfw/include/androidfw/AssetManager2.h @@ -102,9 +102,20 @@ class AssetManager2 { AssetManager2() = default; explicit AssetManager2(AssetManager2&& other) = default; - AssetManager2(ApkAssetsList apk_assets, const ResTable_config& configuration); + struct ScopedOperation { + DISALLOW_COPY_AND_ASSIGN(ScopedOperation); + friend AssetManager2; + const AssetManager2& am_; + ScopedOperation(const AssetManager2& am); + + public: + ~ScopedOperation(); + }; + + [[nodiscard]] ScopedOperation StartOperation() const; + // Sets/resets the underlying ApkAssets for this AssetManager. The ApkAssets // are not owned by the AssetManager, and must have a longer lifetime. // @@ -114,8 +125,9 @@ class AssetManager2 { bool SetApkAssets(ApkAssetsList apk_assets, bool invalidate_caches = true); bool SetApkAssets(std::initializer_list apk_assets, bool invalidate_caches = true); - inline const std::vector& GetApkAssets() const { - return apk_assets_; + const ApkAssetsPtr& GetApkAssets(ApkAssetsCookie cookie) const; + int GetApkAssetsCount() const { + return int(apk_assets_.size()); } // Returns the string pool for the given asset cookie. @@ -426,9 +438,16 @@ class AssetManager2 { base::expected GetBag( uint32_t resid, std::vector& child_resids) const; + // Finish an operation that was running with the current asset manager, and clean up the + // promoted apk assets when the last operation ends. + void FinishOperation() const; + // The ordered list of ApkAssets to search. These are not owned by the AssetManager, and must // have a longer lifetime. - std::vector apk_assets_; + // The second pair element is the promoted version of the assets, that is held for the duration + // of the currently running operation. FinishOperation() clears all promoted assets to make sure + // they can be released when the system needs that. + mutable std::vector> apk_assets_; // DynamicRefTables for shared library package resolution. // These are ordered according to apk_assets_. The mappings may change depending on what is @@ -455,6 +474,10 @@ class AssetManager2 { // Cached set of resolved resource values. mutable std::unordered_map cached_resolved_values_; + // Tracking the number of the started operations running with the current AssetManager. + // Finishing the last one clears all promoted apk assets. + mutable int number_of_running_scoped_operations_ = 0; + // Whether or not to save resource resolution steps bool resource_resolution_logging_enabled_ = false; diff --git a/libs/androidfw/tests/AssetManager2_test.cpp b/libs/androidfw/tests/AssetManager2_test.cpp index 5a5bafdf829a..df3fa02ce44c 100644 --- a/libs/androidfw/tests/AssetManager2_test.cpp +++ b/libs/androidfw/tests/AssetManager2_test.cpp @@ -207,11 +207,11 @@ TEST_F(AssetManager2Test, AssignsOverlayPackageIdLast) { AssetManager2 assetmanager; assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); - auto apk_assets = assetmanager.GetApkAssets(); - ASSERT_EQ(3, apk_assets.size()); - ASSERT_EQ(overlayable_assets_, apk_assets[0].promote()); - ASSERT_EQ(overlay_assets_, apk_assets[1].promote()); - ASSERT_EQ(lib_one_assets_, apk_assets[2].promote()); + ASSERT_EQ(3, assetmanager.GetApkAssetsCount()); + auto op = assetmanager.StartOperation(); + ASSERT_EQ(overlayable_assets_, assetmanager.GetApkAssets(0)); + ASSERT_EQ(overlay_assets_, assetmanager.GetApkAssets(1)); + ASSERT_EQ(lib_one_assets_, assetmanager.GetApkAssets(2)); auto get_first_package_id = [&assetmanager](auto apkAssets) -> uint8_t { return assetmanager.GetAssignedPackageId(apkAssets->GetLoadedArsc()->GetPackages()[0].get()); @@ -834,4 +834,26 @@ TEST_F(AssetManager2Test, GetOverlayablesToString) { std::string::npos); } +TEST_F(AssetManager2Test, GetApkAssets) { + AssetManager2 assetmanager; + assetmanager.SetApkAssets({overlayable_assets_, overlay_assets_, lib_one_assets_}); + + ASSERT_EQ(3, assetmanager.GetApkAssetsCount()); + EXPECT_EQ(1, overlayable_assets_->getStrongCount()); + EXPECT_EQ(1, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); + + { + auto op = assetmanager.StartOperation(); + ASSERT_EQ(overlayable_assets_, assetmanager.GetApkAssets(0)); + ASSERT_EQ(overlay_assets_, assetmanager.GetApkAssets(1)); + EXPECT_EQ(2, overlayable_assets_->getStrongCount()); + EXPECT_EQ(2, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); + } + EXPECT_EQ(1, overlayable_assets_->getStrongCount()); + EXPECT_EQ(1, overlay_assets_->getStrongCount()); + EXPECT_EQ(1, lib_one_assets_->getStrongCount()); +} + } // namespace android diff --git a/libs/androidfw/tests/Idmap_test.cpp b/libs/androidfw/tests/Idmap_test.cpp index 568e041ebe5b..60aa7d88925d 100644 --- a/libs/androidfw/tests/Idmap_test.cpp +++ b/libs/androidfw/tests/Idmap_test.cpp @@ -66,9 +66,9 @@ class IdmapTest : public ::testing::Test { std::string GetStringFromApkAssets(const AssetManager2& asset_manager, const AssetManager2::SelectedValue& value) { - auto assets = asset_manager.GetApkAssets(); + auto op = asset_manager.StartOperation(); const ResStringPool* string_pool = - assets[value.cookie].promote()->GetLoadedArsc()->GetStringPool(); + asset_manager.GetApkAssets(value.cookie)->GetLoadedArsc()->GetStringPool(); return GetStringFromPool(string_pool, value.data); } -- cgit v1.2.3-59-g8ed1b