Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2f271fe4 authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi
Browse files

[res] Don't create extra asset provider when not needed

MultiAssetsProvider is useful to combine a ResourcesLoader with
a file-based provider, but most of the time it's used without
any loaders, and ends up with an EmptyAssetsProvider object that
just wastes cycles and RAM.

This CL optimizes that out, only creating the extra layer when
we have a real loader, and falling back to the only remaining
provider otherwise

Bug: 319137634
Test: build + boot + manual + atest libandroidfw_tests
Flag: EXEMPT optimization
Change-Id: Ibc5ac60adc5e008b70a62481a747758c89083ff9
parent 84f0758b
Loading
Loading
Loading
Loading
+31 −25
Original line number Diff line number Diff line
@@ -111,9 +111,8 @@ static void DeleteGuardedApkAssets(Guarded<AssetManager2::ApkAssetsPtr>& apk_ass
class LoaderAssetsProvider : public AssetsProvider {
 public:
  static std::unique_ptr<AssetsProvider> Create(JNIEnv* env, jobject assets_provider) {
    return (!assets_provider) ? EmptyAssetsProvider::Create()
                              : std::unique_ptr<AssetsProvider>(new LoaderAssetsProvider(
                                    env, assets_provider));
      return std::unique_ptr<AssetsProvider>{
              assets_provider ? new LoaderAssetsProvider(env, assets_provider) : nullptr};
  }

  bool ForEachFile(const std::string& /* root_path */,
@@ -212,7 +211,7 @@ class LoaderAssetsProvider : public AssetsProvider {
    auto string_result = static_cast<jstring>(env->CallObjectMethod(
        assets_provider_, gAssetsProviderOffsets.toString));
    ScopedUtfChars str(env, string_result);
    debug_name_ = std::string(str.c_str(), str.size());
    debug_name_ = std::string(str.c_str());
  }

  // The global reference to the AssetsProvider
@@ -244,7 +243,7 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma
      break;
    case FORMAT_ARSC:
        apk_assets = ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFile(path.c_str()),
                                        std::move(loader_assets),
                                          MultiAssetsProvider::Create(std::move(loader_assets)),
                                          property_flags);
        break;
    case FORMAT_DIRECTORY: {
@@ -316,9 +315,10 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t
        break;
    }
    case FORMAT_ARSC:
      apk_assets = ApkAssets::LoadTable(
          AssetsProvider::CreateAssetFromFd(std::move(dup_fd), nullptr /* path */),
          std::move(loader_assets), property_flags);
        apk_assets = ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFd(std::move(dup_fd),
                                                                            nullptr /* path */),
                                          MultiAssetsProvider::Create(std::move(loader_assets)),
                                          property_flags);
        break;
    default:
      const std::string error_msg = base::StringPrintf("Unsupported format type %d", format);
@@ -386,11 +386,14 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_
        break;
    }
    case FORMAT_ARSC:
      apk_assets = ApkAssets::LoadTable(
          AssetsProvider::CreateAssetFromFd(std::move(dup_fd), nullptr /* path */,
        apk_assets =
                ApkAssets::LoadTable(AssetsProvider::CreateAssetFromFd(std::move(dup_fd),
                                                                       nullptr /* path */,
                                                                       static_cast<off64_t>(offset),
                                            static_cast<off64_t>(length)),
          std::move(loader_assets), property_flags);
                                                                       static_cast<off64_t>(
                                                                               length)),
                                     MultiAssetsProvider::Create(std::move(loader_assets)),
                                     property_flags);
        break;
    default:
      const std::string error_msg = base::StringPrintf("Unsupported format type %d", format);
@@ -408,10 +411,13 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_
}

static jlong NativeLoadEmpty(JNIEnv* env, jclass /*clazz*/, jint flags, jobject assets_provider) {
  auto apk_assets = ApkAssets::Load(LoaderAssetsProvider::Create(env, assets_provider), flags);
    auto apk_assets = ApkAssets::Load(MultiAssetsProvider::Create(
                                              LoaderAssetsProvider::Create(env, assets_provider)),
                                      flags);
    if (apk_assets == nullptr) {
        const std::string error_msg =
        base::StringPrintf("Failed to load empty assets with provider %p", (void*)assets_provider);
                base::StringPrintf("Failed to load empty assets with provider %p",
                                   (void*)assets_provider);
        jniThrowException(env, "java/io/IOException", error_msg.c_str());
        return 0;
    }
+11 −6
Original line number Diff line number Diff line
@@ -24,9 +24,8 @@
#include <ziparchive/zip_archive.h>

namespace android {
namespace {
constexpr const char* kEmptyDebugString = "<empty>";
} // namespace

static constexpr std::string_view kEmptyDebugString = "<empty>";

std::unique_ptr<Asset> AssetsProvider::Open(const std::string& path, Asset::AccessMode mode,
                                            bool* file_exists) const {
@@ -356,8 +355,14 @@ MultiAssetsProvider::MultiAssetsProvider(std::unique_ptr<AssetsProvider>&& prima

std::unique_ptr<AssetsProvider> MultiAssetsProvider::Create(
    std::unique_ptr<AssetsProvider>&& primary, std::unique_ptr<AssetsProvider>&& secondary) {
  if (primary == nullptr || secondary == nullptr) {
    return nullptr;
  if (primary == nullptr && secondary == nullptr) {
    return EmptyAssetsProvider::Create();
  }
  if (!primary) {
    return secondary;
  }
  if (!secondary) {
    return primary;
  }
  return std::unique_ptr<MultiAssetsProvider>(new MultiAssetsProvider(std::move(primary),
                                                                      std::move(secondary)));
@@ -425,7 +430,7 @@ const std::string& EmptyAssetsProvider::GetDebugName() const {
  if (path_.has_value()) {
    return *path_;
  }
  const static std::string kEmpty = kEmptyDebugString;
  constexpr static std::string kEmpty{kEmptyDebugString};
  return kEmpty;
}

+1 −1
Original line number Diff line number Diff line
@@ -164,7 +164,7 @@ struct DirectoryAssetsProvider : public AssetsProvider {
// `secondary` asset provider if the asset cannot be found in the `primary`.
struct MultiAssetsProvider : public AssetsProvider {
  static std::unique_ptr<AssetsProvider> Create(std::unique_ptr<AssetsProvider>&& primary,
                                                std::unique_ptr<AssetsProvider>&& secondary);
                                                std::unique_ptr<AssetsProvider>&& secondary = {});

  bool ForEachFile(const std::string& root_path,
                   base::function_ref<void(StringPiece, FileType)> f) const override;