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

Commit cf6e79f8 authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi Committed by Android (Google) Code Review
Browse files

Revert "Use reference counted pointers for ApkAssets" 

This reverts commit c357f719.

Reason for revert: b/279154343 - performance regression

Change-Id: If2e212d8fc5b9ed8638032a33f450440cbaeceb0
parent c357f719

cmds/idmap2/idmap2/Lookup.cpp

+17 −14
Original line number Original line Diff line number Diff line
@@ -174,7 +174,7 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { 
    return Error("failed to parse config");

    return Error("failed to parse config");

  }

  }





  std::vector<AssetManager2::ApkAssetsPtr> apk_assets;

  std::vector<std::unique_ptr<const ApkAssets>> apk_assets;

  std::string target_path;

  std::string target_path;

  std::string target_package_name;

  std::string target_package_name;

  for (size_t i = 0; i < idmap_paths.size(); i++) {

  for (size_t i = 0; i < idmap_paths.size(); i++) {
@@ -217,9 +217,13 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { 
    apk_assets.push_back(std::move(overlay_apk));

    apk_assets.push_back(std::move(overlay_apk));

  }

  }





  {

  // AssetManager2::SetApkAssets requires raw ApkAssets pointers, not unique_ptrs

    // Make sure |apk_assets| vector outlives the asset manager as it doesn't own the assets.

  std::vector<const ApkAssets*> raw_pointer_apk_assets;

    AssetManager2 am(apk_assets, config);

  std::transform(apk_assets.cbegin(), apk_assets.cend(), std::back_inserter(raw_pointer_apk_assets),

                 [](const auto& p) -> const ApkAssets* { return p.get(); });

  AssetManager2 am;

  am.SetApkAssets(raw_pointer_apk_assets);

  am.SetConfiguration(config);





  const Result<ResourceId> resid = ParseResReference(am, resid_str, target_package_name);

  const Result<ResourceId> resid = ParseResReference(am, resid_str, target_package_name);

  if (!resid) {

  if (!resid) {
@@ -231,7 +235,6 @@ Result<Unit> Lookup(const std::vector<std::string>& args) { 
    return Error(value.GetError(), "resource 0x%08x not found", *resid);

    return Error(value.GetError(), "resource 0x%08x not found", *resid);

  }

  }

  std::cout << *value << std::endl;

  std::cout << *value << std::endl;

  }





  return Unit{};

  return Unit{};

}
 
}

cmds/idmap2/libidmap2/ResourceContainer.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -262,7 +262,7 @@ OverlayData CreateResourceMappingLegacy(const AssetManager2* overlay_am, 
}

}





struct ResState {

struct ResState {

  AssetManager2::ApkAssetsPtr apk_assets;

  std::unique_ptr<ApkAssets> apk_assets;

  const LoadedArsc* arsc;

  const LoadedArsc* arsc;

  const LoadedPackage* package;

  const LoadedPackage* package;

  std::unique_ptr<AssetManager2> am;

  std::unique_ptr<AssetManager2> am;
@@ -284,7 +284,7 @@ struct ResState { 
    }

    }





    state.am = std::make_unique<AssetManager2>();

    state.am = std::make_unique<AssetManager2>();

    if (!state.am->SetApkAssets({state.apk_assets})) {

    if (!state.am->SetApkAssets({state.apk_assets.get()})) {

      return Error("failed to create asset manager");

      return Error("failed to create asset manager");

    }

    }

cmds/idmap2/tests/ResourceUtilsTests.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -38,7 +38,7 @@ class ResourceUtilsTests : public Idmap2Tests { 
    apk_assets_ = ApkAssets::Load(GetTargetApkPath());

    apk_assets_ = ApkAssets::Load(GetTargetApkPath());

    ASSERT_THAT(apk_assets_, NotNull());

    ASSERT_THAT(apk_assets_, NotNull());





    am_.SetApkAssets({apk_assets_});

    am_.SetApkAssets({apk_assets_.get()});

  }

  }





  const AssetManager2& GetAssetManager() {

  const AssetManager2& GetAssetManager() {
@@ -47,7 +47,7 @@ class ResourceUtilsTests : public Idmap2Tests { 




 private:

 private:

  AssetManager2 am_;

  AssetManager2 am_;

  AssetManager2::ApkAssetsPtr apk_assets_;

  std::unique_ptr<const ApkAssets> apk_assets_;

};

};





TEST_F(ResourceUtilsTests, ResToTypeEntryName) {

TEST_F(ResourceUtilsTests, ResToTypeEntryName) {

core/jni/android_content_res_ApkAssets.cpp

+11 −37
Original line number Original line Diff line number Diff line
@@ -74,36 +74,16 @@ enum : format_type_t { 
  FORMAT_DIRECTORY = 3,

  FORMAT_DIRECTORY = 3,

};

};





Guarded<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr) {

Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr) {

  return *reinterpret_cast<Guarded<AssetManager2::ApkAssetsPtr>*>(ptr);

    return *reinterpret_cast<Guarded<std::unique_ptr<const ApkAssets>>*>(ptr);

}

}





static jlong CreateGuardedApkAssets(AssetManager2::ApkAssetsPtr assets) {

static jlong CreateGuardedApkAssets(std::unique_ptr<const ApkAssets> assets) {

  auto guarded_assets = new Guarded<AssetManager2::ApkAssetsPtr>(std::move(assets));

    auto guarded_assets = new Guarded<std::unique_ptr<const ApkAssets>>(std::move(assets));

    return reinterpret_cast<jlong>(guarded_assets);

    return reinterpret_cast<jlong>(guarded_assets);

}

}





static void DeleteGuardedApkAssets(Guarded<AssetManager2::ApkAssetsPtr>& apk_assets) {

static void DeleteGuardedApkAssets(Guarded<std::unique_ptr<const ApkAssets>>& apk_assets) {

  apk_assets.safeDelete([&apk_assets](AssetManager2::ApkAssetsPtr* assets) {

    if (!assets) {

      ALOGE("ApkAssets: Double delete of native assets object %p, ignored", &apk_assets);

    } else if (!*assets) {

      ALOGE("ApkAssets: Empty native assets pointer in native assets object %p", &apk_assets);

    } else {

      // |RefBase| increments |StrongCount| for each |sp<>| instance, and |WeakCount| for

      // both |sp<>| and |wp<>| instances. This means the actual |wp<>| instance count

      // is |WeakCount - StrongCount|.

      const auto useCount = (*assets)->getStrongCount();

      const auto weakCount = (*assets)->getWeakRefs()->getWeakCount() - useCount;

      if (useCount > 1) {

        ALOGE("ApkAssets: Deleting an object '%s' with %d > 1 strong and %d weak references",

              (*assets)->GetDebugName().c_str(), int(useCount), int(weakCount));

      } else if (weakCount > 0) {

        ALOGE("ApkAssets: Deleting an ApkAssets object '%s' with %d weak references",

              (*assets)->GetDebugName().c_str(), int(weakCount));

      }

    }

  });

    delete &apk_assets;

    delete &apk_assets;

}

}
@@ -229,7 +209,7 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma 
  ATRACE_NAME(base::StringPrintf("LoadApkAssets(%s)", path.c_str()).c_str());

  ATRACE_NAME(base::StringPrintf("LoadApkAssets(%s)", path.c_str()).c_str());





  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  AssetManager2::ApkAssetsPtr apk_assets;

  std::unique_ptr<ApkAssets> apk_assets;

  switch (format) {

  switch (format) {

    case FORMAT_APK: {

    case FORMAT_APK: {

        auto assets = MultiAssetsProvider::Create(std::move(loader_assets),

        auto assets = MultiAssetsProvider::Create(std::move(loader_assets),
@@ -289,7 +269,7 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t 
  }

  }





  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  AssetManager2::ApkAssetsPtr apk_assets;

  std::unique_ptr<const ApkAssets> apk_assets;

  switch (format) {

  switch (format) {

    case FORMAT_APK: {

    case FORMAT_APK: {

        auto assets =

        auto assets =
@@ -356,7 +336,7 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ 
  }

  }





  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  auto loader_assets = LoaderAssetsProvider::Create(env, assets_provider);

  AssetManager2::ApkAssetsPtr apk_assets;

  std::unique_ptr<const ApkAssets> apk_assets;

  switch (format) {

  switch (format) {

    case FORMAT_APK: {

    case FORMAT_APK: {

        auto assets =

        auto assets =
@@ -394,12 +374,6 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ 




static jlong NativeLoadEmpty(JNIEnv* env, jclass /*clazz*/, jint flags, jobject assets_provider) {

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(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);

    jniThrowException(env, "java/io/IOException", error_msg.c_str());

    return 0;

  }

  return CreateGuardedApkAssets(std::move(apk_assets));

  return CreateGuardedApkAssets(std::move(apk_assets));

}

}

core/jni/android_content_res_ApkAssets.h

+2 −2
Original line number Original line Diff line number Diff line
@@ -18,13 +18,13 @@ 
#define ANDROID_CONTENT_RES_APKASSETS_H

#define ANDROID_CONTENT_RES_APKASSETS_H





#include "androidfw/ApkAssets.h"

#include "androidfw/ApkAssets.h"

#include "androidfw/AssetManager2.h"

#include "androidfw/MutexGuard.h"

#include "androidfw/MutexGuard.h"



#include "jni.h"

#include "jni.h"





namespace android {

namespace android {





Guarded<AssetManager2::ApkAssetsPtr>& ApkAssetsFromLong(jlong ptr);

Guarded<std::unique_ptr<const ApkAssets>>& ApkAssetsFromLong(jlong ptr);





} // namespace android

} // namespace android