From 7d53f19089829409075d8ad3dbf3447e80f0f2ac Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Fri, 24 Apr 2020 17:45:25 -0700 Subject: [PATCH] Reduce OMS start time This change attempts to reduce the amount of time initializing the OMS. The change: 1) Reduces the amount of time between subsequent attempts to connect to the idmap2d service when it is not alive. 2) Caches package infos retrieved from getOverlayPackages 3) Caches the crc of the android package to preventing having to retrieve it for every overlay package targeting the framework This chance reduced OMS start up time from ~500ms to ~100ms on a Pixel 3. If the idmap2d service is running when system sever starts then start up time will be around ~70ms. Bug: 151481016 Test: adb shell stop && adb shell start && [capture trace] Change-Id: I6137c385baf099413b62e98557419fffb9fd2d93 --- cmds/idmap2/idmap2d/Idmap2Service.cpp | 32 ++++++++++++++- cmds/idmap2/idmap2d/Idmap2Service.h | 4 ++ cmds/idmap2/include/idmap2/Idmap.h | 5 +++ cmds/idmap2/libidmap2/Idmap.cpp | 40 ++++++++++--------- .../com/android/server/om/IdmapDaemon.java | 2 +- .../server/om/OverlayManagerService.java | 6 ++- 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/cmds/idmap2/idmap2d/Idmap2Service.cpp b/cmds/idmap2/idmap2d/Idmap2Service.cpp index 75fc7f714ce32..a93184ff47875 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.cpp +++ b/cmds/idmap2/idmap2d/Idmap2Service.cpp @@ -34,11 +34,13 @@ #include "idmap2/FileUtils.h" #include "idmap2/Idmap.h" #include "idmap2/SysTrace.h" +#include "idmap2/ZipFile.h" #include "utils/String8.h" using android::IPCThreadState; using android::binder::Status; using android::idmap2::BinaryStreamVisitor; +using android::idmap2::GetPackageCrc; using android::idmap2::Idmap; using android::idmap2::IdmapHeader; using android::idmap2::utils::kIdmapCacheDir; @@ -49,6 +51,8 @@ using PolicyBitmask = android::ResTable_overlayable_policy_header::PolicyBitmask namespace { +constexpr const char* kFrameworkPath = "/system/framework/framework-res.apk"; + Status ok() { return Status::ok(); } @@ -109,8 +113,32 @@ Status Idmap2Service::verifyIdmap(const std::string& target_apk_path, return error("failed to parse idmap header"); } - *_aidl_return = - strcmp(header->GetTargetPath().data(), target_apk_path.data()) == 0 && header->IsUpToDate(); + if (strcmp(header->GetTargetPath().data(), target_apk_path.data()) != 0) { + *_aidl_return = false; + return ok(); + } + + if (target_apk_path != kFrameworkPath) { + *_aidl_return = (bool) header->IsUpToDate(); + } else { + if (!android_crc_) { + // Loading the framework zip can take several milliseconds. Cache the crc of the framework + // resource APK to reduce repeated work during boot. + const auto target_zip = idmap2::ZipFile::Open(target_apk_path); + if (!target_zip) { + return error(base::StringPrintf("failed to open target %s", target_apk_path.c_str())); + } + + const auto target_crc = GetPackageCrc(*target_zip); + if (!target_crc) { + return error(target_crc.GetErrorMessage()); + } + + android_crc_ = *target_crc; + } + + *_aidl_return = (bool) header->IsUpToDate(android_crc_.value()); + } // TODO(b/119328308): Check that the set of fulfilled policies of the overlay has not changed return ok(); diff --git a/cmds/idmap2/idmap2d/Idmap2Service.h b/cmds/idmap2/idmap2d/Idmap2Service.h index 0ed55a1bb6a4a..55fb5ad077819 100644 --- a/cmds/idmap2/idmap2d/Idmap2Service.h +++ b/cmds/idmap2/idmap2d/Idmap2Service.h @@ -46,6 +46,10 @@ class Idmap2Service : public BinderService, public BnIdmap2 { const std::string& overlay_apk_path, int32_t fulfilled_policies, bool enforce_overlayable, int32_t user_id, aidl::nullable* _aidl_return) override; + + private: + // Cache the crc of the android framework package since the crc cannot change without a reboot. + std::optional android_crc_; }; } // namespace android::os diff --git a/cmds/idmap2/include/idmap2/Idmap.h b/cmds/idmap2/include/idmap2/Idmap.h index 2e4836e297ec2..77a7b30a230e0 100644 --- a/cmds/idmap2/include/idmap2/Idmap.h +++ b/cmds/idmap2/include/idmap2/Idmap.h @@ -74,6 +74,7 @@ #include "androidfw/ResourceTypes.h" #include "androidfw/StringPiece.h" #include "idmap2/ResourceMapping.h" +#include "idmap2/ZipFile.h" namespace android::idmap2 { @@ -93,6 +94,9 @@ static constexpr const uint32_t kIdmapCurrentVersion = android::kIdmapCurrentVer // terminating null) static constexpr const size_t kIdmapStringLength = 256; +// Retrieves a crc generated using all of the files within the zip that can affect idmap generation. +Result GetPackageCrc(const ZipFile& zip_info); + class IdmapHeader { public: static std::unique_ptr FromBinaryStream(std::istream& stream); @@ -129,6 +133,7 @@ class IdmapHeader { // field *must* be incremented. Because of this, we know that if the idmap // header is up-to-date the entire file is up-to-date. Result IsUpToDate() const; + Result IsUpToDate(uint32_t target_crc_) const; void accept(Visitor* v) const; diff --git a/cmds/idmap2/libidmap2/Idmap.cpp b/cmds/idmap2/libidmap2/Idmap.cpp index 7f2cd9596c956..706b842b3b478 100644 --- a/cmds/idmap2/libidmap2/Idmap.cpp +++ b/cmds/idmap2/libidmap2/Idmap.cpp @@ -100,7 +100,9 @@ Result ReadString(std::istream& stream) { return buf; } -Result GetCrc(const ZipFile& zip) { +} // namespace + +Result GetPackageCrc(const ZipFile& zip) { const Result a = zip.Crc("resources.arsc"); const Result b = zip.Crc("AndroidManifest.xml"); return a && b @@ -108,8 +110,6 @@ Result GetCrc(const ZipFile& zip) { : Error("failed to get CRC for \"%s\"", a ? "AndroidManifest.xml" : "resources.arsc"); } -} // namespace - std::unique_ptr IdmapHeader::FromBinaryStream(std::istream& stream) { std::unique_ptr idmap_header(new IdmapHeader()); @@ -130,6 +130,20 @@ std::unique_ptr IdmapHeader::FromBinaryStream(std::istream& s } Result IdmapHeader::IsUpToDate() const { + const std::unique_ptr target_zip = ZipFile::Open(target_path_); + if (!target_zip) { + return Error("failed to open target %s", GetTargetPath().to_string().c_str()); + } + + Result target_crc = GetPackageCrc(*target_zip); + if (!target_crc) { + return Error("failed to get target crc"); + } + + return IsUpToDate(*target_crc); +} + +Result IdmapHeader::IsUpToDate(uint32_t target_crc) const { if (magic_ != kIdmapMagic) { return Error("bad magic: actual 0x%08x, expected 0x%08x", magic_, kIdmapMagic); } @@ -138,19 +152,9 @@ Result IdmapHeader::IsUpToDate() const { return Error("bad version: actual 0x%08x, expected 0x%08x", version_, kIdmapCurrentVersion); } - const std::unique_ptr target_zip = ZipFile::Open(target_path_); - if (!target_zip) { - return Error("failed to open target %s", GetTargetPath().to_string().c_str()); - } - - Result target_crc = GetCrc(*target_zip); - if (!target_crc) { - return Error("failed to get target crc"); - } - - if (target_crc_ != *target_crc) { + if (target_crc_ != target_crc) { return Error("bad target crc: idmap version 0x%08x, file system version 0x%08x", target_crc_, - *target_crc); + target_crc); } const std::unique_ptr overlay_zip = ZipFile::Open(overlay_path_); @@ -158,7 +162,7 @@ Result IdmapHeader::IsUpToDate() const { return Error("failed to open overlay %s", GetOverlayPath().to_string().c_str()); } - Result overlay_crc = GetCrc(*overlay_zip); + Result overlay_crc = GetPackageCrc(*overlay_zip); if (!overlay_crc) { return Error("failed to get overlay crc"); } @@ -304,13 +308,13 @@ Result> Idmap::FromApkAssets(const ApkAssets& targe header->magic_ = kIdmapMagic; header->version_ = kIdmapCurrentVersion; - Result crc = GetCrc(*target_zip); + Result crc = GetPackageCrc(*target_zip); if (!crc) { return Error(crc.GetError(), "failed to get zip CRC for target"); } header->target_crc_ = *crc; - crc = GetCrc(*overlay_zip); + crc = GetPackageCrc(*overlay_zip); if (!crc) { return Error(crc.GetError(), "failed to get zip CRC for overlay"); } diff --git a/services/core/java/com/android/server/om/IdmapDaemon.java b/services/core/java/com/android/server/om/IdmapDaemon.java index 7df8fc7e34edd..910ed44df0f87 100644 --- a/services/core/java/com/android/server/om/IdmapDaemon.java +++ b/services/core/java/com/android/server/om/IdmapDaemon.java @@ -44,7 +44,7 @@ class IdmapDaemon { // The amount of time in milliseconds to wait when attempting to connect to idmap service. private static final int SERVICE_CONNECT_TIMEOUT_MS = 5000; - private static final int SERVICE_CONNECT_INTERVAL_SLEEP_MS = 200; + private static final int SERVICE_CONNECT_INTERVAL_SLEEP_MS = 5; private static final String IDMAP_DAEMON = "idmap2d"; diff --git a/services/core/java/com/android/server/om/OverlayManagerService.java b/services/core/java/com/android/server/om/OverlayManagerService.java index c81f7cd4a8b48..fedadd1b0e93e 100644 --- a/services/core/java/com/android/server/om/OverlayManagerService.java +++ b/services/core/java/com/android/server/om/OverlayManagerService.java @@ -1120,7 +1120,11 @@ public final class OverlayManagerService extends SystemService { @Override public List getOverlayPackages(final int userId) { - return mPackageManagerInternal.getOverlayPackages(userId); + final List overlays = mPackageManagerInternal.getOverlayPackages(userId); + for (final PackageInfo info : overlays) { + cachePackageInfo(info.packageName, userId, info); + } + return overlays; } @Nullable