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

Commit 30080e2f authored by Adam Lesinski's avatar Adam Lesinski
Browse files

AssetManager2: Improve Theme performance

This change brings Theme ApplyStyle down to 2x the original performance
and Theme attribute retrieval to less than the original performance.
Yay!

Benchmarks ran on marlin-eng
----------------------------------------------------------------------
Benchmark                               Time           CPU Iterations
----------------------------------------------------------------------
BM_ThemeApplyStyleFramework          8540 ns       8500 ns      82105
BM_ThemeApplyStyleFrameworkOld       5280 ns       5258 ns     148849
BM_ThemeGetAttribute                    8 ns          8 ns   88388549
BM_ThemeGetAttributeOld                11 ns         11 ns   63394463

ApplyStyle still takes some time, and the weird thing is that if I
switch the data structure of ThemeType to use an
std::vector<ThemeEntry>, the performance becomes better than the
original implementation! The issue is that std::vector<T> takes up 24
bytes, which would make Themes take up 8 more bytes per ThemeType, which
is unacceptable. Still trying to isolate where the performance gain is
coming from.

Test: make libandroidfw_tests && $ANDROID_BUILD_TOP/out/host/<host>/nativetest64/libandroidfw_tests/libandroidfw_tests
Test: make libandroidfw_benchmarks && adb sync system && adb sync data && adb shell /data/benchmarktest64/libandroidfw_benchmarks/libandroidfw_benchmarks
Change-Id: I0e7a756afd44b6aac1521e69c2b907258c262d3e
parent 8a0f0ed4
Loading
Loading
Loading
Loading
+142 −135
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@

#include "androidfw/AssetManager2.h"

#include <iterator>
#include <set>

#include "android-base/logging.h"
@@ -506,10 +507,17 @@ const ResolvedBag* AssetManager2::GetBag(uint32_t resid) {
        }
      }
      new_entry->cookie = cookie;
      new_entry->value.copyFrom_dtoh(map_entry->value);
      new_entry->key = new_key;
      new_entry->key_pool = nullptr;
      new_entry->type_pool = nullptr;
      new_entry->value.copyFrom_dtoh(map_entry->value);
      status_t err = entry.dynamic_ref_table->lookupResourceValue(&new_entry->value);
      if (err != NO_ERROR) {
        LOG(ERROR) << base::StringPrintf(
            "Failed to resolve value t=0x%02x d=0x%08x for key 0x%08x.", new_entry->value.dataType,
            new_entry->value.data, new_key);
        return nullptr;
      }
      ++new_entry;
    }
    new_bag->type_spec_flags = flags;
@@ -557,10 +565,17 @@ const ResolvedBag* AssetManager2::GetBag(uint32_t resid) {
      // Use the child key if it comes before the parent
      // or is equal to the parent (overrides).
      new_entry->cookie = cookie;
      new_entry->value.copyFrom_dtoh(map_entry->value);
      new_entry->key = child_key;
      new_entry->key_pool = nullptr;
      new_entry->type_pool = nullptr;
      new_entry->value.copyFrom_dtoh(map_entry->value);
      status_t err = entry.dynamic_ref_table->lookupResourceValue(&new_entry->value);
      if (err != NO_ERROR) {
        LOG(ERROR) << base::StringPrintf(
            "Failed to resolve value t=0x%02x d=0x%08x for key 0x%08x.", new_entry->value.dataType,
            new_entry->value.data, child_key);
        return nullptr;
      }
      ++map_entry;
    } else {
      // Take the parent entry as-is.
@@ -585,10 +600,16 @@ const ResolvedBag* AssetManager2::GetBag(uint32_t resid) {
      }
    }
    new_entry->cookie = cookie;
    new_entry->value.copyFrom_dtoh(map_entry->value);
    new_entry->key = new_key;
    new_entry->key_pool = nullptr;
    new_entry->type_pool = nullptr;
    new_entry->value.copyFrom_dtoh(map_entry->value);
    status_t err = entry.dynamic_ref_table->lookupResourceValue(&new_entry->value);
    if (err != NO_ERROR) {
      LOG(ERROR) << base::StringPrintf("Failed to resolve value t=0x%02x d=0x%08x for key 0x%08x.",
                                       new_entry->value.dataType, new_entry->value.data, new_key);
      return nullptr;
    }
    ++map_entry;
    ++new_entry;
  }
@@ -701,7 +722,37 @@ void AssetManager2::InvalidateCaches(uint32_t diff) {
  }
}

std::unique_ptr<Theme> AssetManager2::NewTheme() { return std::unique_ptr<Theme>(new Theme(this)); }
std::unique_ptr<Theme> AssetManager2::NewTheme() {
  return std::unique_ptr<Theme>(new Theme(this));
}

Theme::Theme(AssetManager2* asset_manager) : asset_manager_(asset_manager) {
}

Theme::~Theme() = default;

namespace {

struct ThemeEntry {
  ApkAssetsCookie cookie;
  uint32_t type_spec_flags;
  Res_value value;
};

struct ThemeType {
  int entry_count;
  ThemeEntry entries[0];
};

constexpr size_t kTypeCount = std::numeric_limits<uint8_t>::max() + 1;

}  // namespace

struct Theme::Package {
  // Each element of Type will be a dynamically sized object
  // allocated to have the entries stored contiguously with the Type.
  std::array<util::unique_cptr<ThemeType>, kTypeCount> types;
};

bool Theme::ApplyStyle(uint32_t resid, bool force) {
  ATRACE_CALL();
@@ -714,71 +765,69 @@ bool Theme::ApplyStyle(uint32_t resid, bool force) {
  // Merge the flags from this style.
  type_spec_flags_ |= bag->type_spec_flags;

  // On the first iteration, verify the attribute IDs and
  // update the entry count in each type.
  const auto bag_iter_end = end(bag);
  for (auto bag_iter = begin(bag); bag_iter != bag_iter_end; ++bag_iter) {
  int last_type_idx = -1;
  int last_package_idx = -1;
  Package* last_package = nullptr;
  ThemeType* last_type = nullptr;

  // Iterate backwards, because each bag is sorted in ascending key ID order, meaning we will only
  // need to perform one resize per type.
  using reverse_bag_iterator = std::reverse_iterator<const ResolvedBag::Entry*>;
  const auto bag_iter_end = reverse_bag_iterator(begin(bag));
  for (auto bag_iter = reverse_bag_iterator(end(bag)); bag_iter != bag_iter_end; ++bag_iter) {
    const uint32_t attr_resid = bag_iter->key;

    // If the resource ID passed in is not a style, the key can be
    // some other identifier that is not a resource ID.
    // If the resource ID passed in is not a style, the key can be some other identifier that is not
    // a resource ID. We should fail fast instead of operating with strange resource IDs.
    if (!is_valid_resid(attr_resid)) {
      return false;
    }

    const uint32_t package_idx = get_package_id(attr_resid);

    // The type ID is 1-based, so subtract 1 to get an index.
    const uint32_t type_idx = get_type_id(attr_resid) - 1;
    const uint32_t entry_idx = get_entry_id(attr_resid);
    // We don't use the 0-based index for the type so that we can avoid doing ID validation
    // upon lookup. Instead, we keep space for the type ID 0 in our data structures. Since
    // the construction of this type is guarded with a resource ID check, it will never be
    // populated, and querying type ID 0 will always fail.
    const int package_idx = get_package_id(attr_resid);
    const int type_idx = get_type_id(attr_resid);
    const int entry_idx = get_entry_id(attr_resid);

    if (last_package_idx != package_idx) {
      std::unique_ptr<Package>& package = packages_[package_idx];
      if (package == nullptr) {
        package.reset(new Package());
      }
      last_package_idx = package_idx;
      last_package = package.get();
      last_type_idx = -1;
    }

    util::unique_cptr<Type>& type = package->types[type_idx];
    if (last_type_idx != type_idx) {
      util::unique_cptr<ThemeType>& type = last_package->types[type_idx];
      if (type == nullptr) {
      // Set the initial capacity to take up a total amount of 1024 bytes.
      constexpr uint32_t kInitialCapacity = (1024u - sizeof(Type)) / sizeof(Entry);
      const uint32_t initial_capacity = std::max(entry_idx, kInitialCapacity);
      type.reset(
          reinterpret_cast<Type*>(calloc(sizeof(Type) + (initial_capacity * sizeof(Entry)), 1)));
      type->entry_capacity = initial_capacity;
    }

    // Set the entry_count to include this entry. We will populate
    // and resize the array as necessary in the next pass.
    if (entry_idx + 1 > type->entry_count) {
      // Increase the entry count to include this.
        // Allocate enough memory to contain this entry_idx. Since we're iterating in reverse over
        // a sorted list of attributes, this shouldn't be resized again during this method call.
        type.reset(reinterpret_cast<ThemeType*>(
            calloc(sizeof(ThemeType) + (entry_idx + 1) * sizeof(ThemeEntry), 1)));
        type->entry_count = entry_idx + 1;
      } else if (entry_idx >= type->entry_count) {
        // Reallocate the memory to contain this entry_idx. Since we're iterating in reverse over
        // a sorted list of attributes, this shouldn't be resized again during this method call.
        const int new_count = entry_idx + 1;
        type.reset(reinterpret_cast<ThemeType*>(
            realloc(type.release(), sizeof(ThemeType) + (new_count * sizeof(ThemeEntry)))));

        // Clear out the newly allocated space (which isn't zeroed).
        memset(type->entries + type->entry_count, 0,
               (new_count - type->entry_count) * sizeof(ThemeEntry));
        type->entry_count = new_count;
      }
      last_type_idx = type_idx;
      last_type = type.get();
    }

  // On the second pass, we will realloc to fit the entry counts
  // and populate the structures.
  for (auto bag_iter = begin(bag); bag_iter != bag_iter_end; ++bag_iter) {
    const uint32_t attr_resid = bag_iter->key;
    const uint32_t package_idx = get_package_id(attr_resid);
    const uint32_t type_idx = get_type_id(attr_resid) - 1;
    const uint32_t entry_idx = get_entry_id(attr_resid);
    Package* package = packages_[package_idx].get();
    util::unique_cptr<Type>& type = package->types[type_idx];
    if (type->entry_count != type->entry_capacity) {
      // Resize to fit the actual entries that will be included.
      Type* type_ptr = type.release();
      type.reset(reinterpret_cast<Type*>(
          realloc(type_ptr, sizeof(Type) + (type_ptr->entry_count * sizeof(Entry)))));
      if (type->entry_capacity < type->entry_count) {
        // Clear the newly allocated memory (which does not get zero initialized).
        // We need to do this because we |= type_spec_flags.
        memset(type->entries + type->entry_capacity, 0,
               sizeof(Entry) * (type->entry_count - type->entry_capacity));
      }
      type->entry_capacity = type->entry_count;
    }
    Entry& entry = type->entries[entry_idx];
    if (force || entry.value.dataType == Res_value::TYPE_NULL) {
    ThemeEntry& entry = last_type->entries[entry_idx];
    if (force || (entry.value.dataType == Res_value::TYPE_NULL &&
                  entry.value.data != Res_value::DATA_NULL_EMPTY)) {
      entry.cookie = bag_iter->cookie;
      entry.type_spec_flags |= bag->type_spec_flags;
      entry.value = bag_iter->value;
@@ -789,88 +838,46 @@ bool Theme::ApplyStyle(uint32_t resid, bool force) {

ApkAssetsCookie Theme::GetAttribute(uint32_t resid, Res_value* out_value,
                                    uint32_t* out_flags) const {
  constexpr const int kMaxIterations = 20;
  int cnt = 20;

  uint32_t type_spec_flags = 0u;

  for (int iterations_left = kMaxIterations; iterations_left > 0; iterations_left--) {
    if (!is_valid_resid(resid)) {
      return kInvalidCookie;
    }

    const uint32_t package_idx = get_package_id(resid);

    // Type ID is 1-based, subtract 1 to get the index.
    const uint32_t type_idx = get_type_id(resid) - 1;
    const uint32_t entry_idx = get_entry_id(resid);

  do {
    const int package_idx = get_package_id(resid);
    const Package* package = packages_[package_idx].get();
    if (package == nullptr) {
      return kInvalidCookie;
    }

    const Type* type = package->types[type_idx].get();
    if (type == nullptr) {
      return kInvalidCookie;
    }

    if (entry_idx >= type->entry_count) {
      return kInvalidCookie;
    }

    const Entry& entry = type->entries[entry_idx];
    if (package != nullptr) {
      // The themes are constructed with a 1-based type ID, so no need to decrement here.
      const int type_idx = get_type_id(resid);
      const ThemeType* type = package->types[type_idx].get();
      if (type != nullptr) {
        const int entry_idx = get_entry_id(resid);
        if (entry_idx < type->entry_count) {
          const ThemeEntry& entry = type->entries[entry_idx];
          type_spec_flags |= entry.type_spec_flags;

    switch (entry.value.dataType) {
      case Res_value::TYPE_NULL:
        return kInvalidCookie;

      case Res_value::TYPE_ATTRIBUTE:
        resid = entry.value.data;
        break;

      case Res_value::TYPE_DYNAMIC_ATTRIBUTE: {
        // Resolve the dynamic attribute to a normal attribute
        // (with the right package ID).
          if (entry.value.dataType == Res_value::TYPE_ATTRIBUTE) {
            if (cnt > 0) {
              cnt--;
              resid = entry.value.data;
        const DynamicRefTable* ref_table =
            asset_manager_->GetDynamicRefTableForPackage(package_idx);
        if (ref_table == nullptr || ref_table->lookupResourceId(&resid) != NO_ERROR) {
          LOG(ERROR) << base::StringPrintf("Failed to resolve dynamic attribute 0x%08x", resid);
          return kInvalidCookie;
              continue;
            }
      } break;

      case Res_value::TYPE_DYNAMIC_REFERENCE: {
        // Resolve the dynamic reference to a normal reference
        // (with the right package ID).
        out_value->dataType = Res_value::TYPE_REFERENCE;
        out_value->data = entry.value.data;
        const DynamicRefTable* ref_table =
            asset_manager_->GetDynamicRefTableForPackage(package_idx);
        if (ref_table == nullptr || ref_table->lookupResourceId(&out_value->data) != NO_ERROR) {
          LOG(ERROR) << base::StringPrintf("Failed to resolve dynamic reference 0x%08x",
                                           out_value->data);
            return kInvalidCookie;
          }

        if (out_flags != nullptr) {
          *out_flags = type_spec_flags;
        }
        return entry.cookie;
          // @null is different than @empty.
          if (entry.value.dataType == Res_value::TYPE_NULL &&
              entry.value.data != Res_value::DATA_NULL_EMPTY) {
            return kInvalidCookie;
          }

      default:
          *out_value = entry.value;
        if (out_flags != nullptr) {
          *out_flags = type_spec_flags;
        }
          return entry.cookie;
        }
      }

  LOG(WARNING) << base::StringPrintf("Too many (%d) attribute references, stopped at: 0x%08x",
                                     kMaxIterations, resid);
    }
    break;
  } while (true);
  return kInvalidCookie;
}

@@ -923,7 +930,7 @@ bool Theme::SetTo(const Theme& o) {
    }

    for (size_t t = 0; t < package->types.size(); t++) {
      const Type* type = package->types[t].get();
      const ThemeType* type = package->types[t].get();
      if (type == nullptr) {
        // The other theme doesn't have this type, clear ours.
        packages_[p]->types[t].reset();
@@ -931,10 +938,10 @@ bool Theme::SetTo(const Theme& o) {
      }

      // Create a new type and update it to theirs.
      const size_t type_alloc_size = sizeof(Type) + (type->entry_capacity * sizeof(Entry));
      const size_t type_alloc_size = sizeof(ThemeType) + (type->entry_count * sizeof(ThemeEntry));
      void* copied_data = malloc(type_alloc_size);
      memcpy(copied_data, type, type_alloc_size);
      packages_[p]->types[t].reset(reinterpret_cast<Type*>(copied_data));
      packages_[p]->types[t].reset(reinterpret_cast<ThemeType*>(copied_data));
    }
  }
  return true;
+28 −37
Original line number Diff line number Diff line
@@ -302,6 +302,8 @@ class Theme {
  friend class AssetManager2;

 public:
  ~Theme();

  // Applies the style identified by `resid` to this theme. This can be called
  // multiple times with different styles. By default, any theme attributes that
  // are already defined before this call are not overridden. If `force` is set
@@ -316,27 +318,31 @@ class Theme {

  void Clear();

  inline const AssetManager2* GetAssetManager() const { return asset_manager_; }
  inline const AssetManager2* GetAssetManager() const {
    return asset_manager_;
  }

  inline AssetManager2* GetAssetManager() { return asset_manager_; }
  inline AssetManager2* GetAssetManager() {
    return asset_manager_;
  }

  // Returns a bit mask of configuration changes that will impact this
  // theme (and thus require completely reloading it).
  inline uint32_t GetChangingConfigurations() const { return type_spec_flags_; }
  inline uint32_t GetChangingConfigurations() const {
    return type_spec_flags_;
  }

  // Retrieve a value in the theme. If the theme defines this value,
  // returns an asset cookie indicating which ApkAssets it came from
  // and populates `out_value` with the value. If `out_flags` is non-null,
  // populates it with a bitmask of the configuration axis the resource
  // varies with.
  // Retrieve a value in the theme. If the theme defines this value, returns an asset cookie
  // indicating which ApkAssets it came from and populates `out_value` with the value.
  // `out_flags` is populated with a bitmask of the configuration axis with which the resource
  // varies.
  //
  // If the attribute is not found, returns kInvalidCookie.
  //
  // NOTE: This function does not do reference traversal. If you want
  // to follow references to other resources to get the "real" value to
  // use, you need to call ResolveReference() after this function.
  ApkAssetsCookie GetAttribute(uint32_t resid, Res_value* out_value,
                               uint32_t* out_flags = nullptr) const;
  // NOTE: This function does not do reference traversal. If you want to follow references to other
  // resources to get the "real" value to use, you need to call ResolveReference() after this
  // function.
  ApkAssetsCookie GetAttribute(uint32_t resid, Res_value* out_value, uint32_t* out_flags) const;

  // This is like AssetManager2::ResolveReference(), but also takes
  // care of resolving attribute references to the theme.
@@ -349,36 +355,21 @@ class Theme {
  DISALLOW_COPY_AND_ASSIGN(Theme);

  // Called by AssetManager2.
  explicit inline Theme(AssetManager2* asset_manager) : asset_manager_(asset_manager) {}

  struct Entry {
    ApkAssetsCookie cookie;
    uint32_t type_spec_flags;
    Res_value value;
  };

  struct Type {
    // Use uint32_t for fewer cycles when loading from memory.
    uint32_t entry_count;
    uint32_t entry_capacity;
    Entry entries[0];
  };

  static constexpr const size_t kPackageCount = std::numeric_limits<uint8_t>::max() + 1;
  static constexpr const size_t kTypeCount = std::numeric_limits<uint8_t>::max() + 1;

  struct Package {
    // Each element of Type will be a dynamically sized object
    // allocated to have the entries stored contiguously with the Type.
    std::array<util::unique_cptr<Type>, kTypeCount> types;
  };
  explicit Theme(AssetManager2* asset_manager);

  AssetManager2* asset_manager_;
  uint32_t type_spec_flags_ = 0u;

  // Defined in the cpp.
  struct Package;

  constexpr static size_t kPackageCount = std::numeric_limits<uint8_t>::max() + 1;
  std::array<std::unique_ptr<Package>, kPackageCount> packages_;
};

inline const ResolvedBag::Entry* begin(const ResolvedBag* bag) { return bag->entries; }
inline const ResolvedBag::Entry* begin(const ResolvedBag* bag) {
  return bag->entries;
}

inline const ResolvedBag::Entry* end(const ResolvedBag* bag) {
  return bag->entries + bag->entry_count;
+3 −1
Original line number Diff line number Diff line
@@ -40,7 +40,9 @@ inline uint8_t get_type_id(uint32_t resid) {
  return static_cast<uint8_t>((resid >> 16) & 0x000000ffu);
}

inline uint16_t get_entry_id(uint32_t resid) { return static_cast<uint16_t>(resid & 0x0000ffffu); }
inline uint16_t get_entry_id(uint32_t resid) {
  return static_cast<uint16_t>(resid & 0x0000ffffu);
}

inline bool is_internal_resid(uint32_t resid) {
  return (resid & 0xffff0000u) != 0 && (resid & 0x00ff0000u) == 0;
+12 −0
Original line number Diff line number Diff line
@@ -128,6 +128,18 @@ TEST_F(ThemeTest, SingleThemeWithParent) {
  EXPECT_EQ(static_cast<uint32_t>(ResTable_typeSpec::SPEC_PUBLIC), flags);
}

TEST_F(ThemeTest, TryToUseBadResourceId) {
  AssetManager2 assetmanager;
  assetmanager.SetApkAssets({style_assets_.get()});

  std::unique_ptr<Theme> theme = assetmanager.NewTheme();
  ASSERT_TRUE(theme->ApplyStyle(app::R::style::StyleTwo));

  Res_value value;
  uint32_t flags;
  ASSERT_EQ(kInvalidCookie, theme->GetAttribute(0x7f000001, &value, &flags));
}

TEST_F(ThemeTest, MultipleThemesOverlaidNotForce) {
  AssetManager2 assetmanager;
  assetmanager.SetApkAssets({style_assets_.get()});