[aapt2] Fix the sorting in ResourceTable for flagged entries
+ a few C++ style fixes that are supposed to be noops, but with
the existing perf test regression record I'm not sure anymore
Flag: EXEMPT bugfix
Test: build + boot + aapt2_tests
Change-Id: I918448ddc3840e1a180466d794eb81e8ec4b9640
diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp
index 7a4f40e..9751459 100644
--- a/tools/aapt2/ResourceTable.cpp
+++ b/tools/aapt2/ResourceTable.cpp
@@ -50,21 +50,21 @@
template <typename T>
bool less_than_struct_with_name(const std::unique_ptr<T>& lhs, StringPiece rhs) {
- return lhs->name.compare(0, lhs->name.size(), rhs.data(), rhs.size()) < 0;
+ return lhs->name < rhs;
}
template <typename T>
bool greater_than_struct_with_name(StringPiece lhs, const std::unique_ptr<T>& rhs) {
- return rhs->name.compare(0, rhs->name.size(), lhs.data(), lhs.size()) > 0;
+ return rhs->name > lhs;
}
template <typename T>
struct NameEqualRange {
bool operator()(const std::unique_ptr<T>& lhs, StringPiece rhs) const {
- return less_than_struct_with_name<T>(lhs, rhs);
+ return less_than_struct_with_name(lhs, rhs);
}
bool operator()(StringPiece lhs, const std::unique_ptr<T>& rhs) const {
- return greater_than_struct_with_name<T>(lhs, rhs);
+ return greater_than_struct_with_name(lhs, rhs);
}
};
@@ -74,7 +74,7 @@
if (lhs.id != rhs.second) {
return lhs.id < rhs.second;
}
- return lhs.name.compare(0, lhs.name.size(), rhs.first.data(), rhs.first.size()) < 0;
+ return lhs.name < rhs.first;
}
template <typename T, typename Func, typename Elements>
@@ -90,14 +90,16 @@
StringPiece product;
};
-template <typename T>
-bool lt_config_key_ref(const T& lhs, const ConfigKey& rhs) {
- int cmp = lhs->config.compare(*rhs.config);
- if (cmp == 0) {
- cmp = StringPiece(lhs->product).compare(rhs.product);
+struct lt_config_key_ref {
+ template <typename T>
+ bool operator()(const T& lhs, const ConfigKey& rhs) const noexcept {
+ int cmp = lhs->config.compare(*rhs.config);
+ if (cmp == 0) {
+ cmp = lhs->product.compare(rhs.product);
+ }
+ return cmp < 0;
}
- return cmp < 0;
-}
+};
} // namespace
@@ -159,10 +161,10 @@
ResourceConfigValue* ResourceEntry::FindValue(const ConfigDescription& config,
android::StringPiece product) {
auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product},
- lt_config_key_ref<std::unique_ptr<ResourceConfigValue>>);
+ lt_config_key_ref());
if (iter != values.end()) {
ResourceConfigValue* value = iter->get();
- if (value->config == config && StringPiece(value->product) == product) {
+ if (value->config == config && value->product == product) {
return value;
}
}
@@ -172,10 +174,10 @@
const ResourceConfigValue* ResourceEntry::FindValue(const android::ConfigDescription& config,
android::StringPiece product) const {
auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product},
- lt_config_key_ref<std::unique_ptr<ResourceConfigValue>>);
+ lt_config_key_ref());
if (iter != values.end()) {
ResourceConfigValue* value = iter->get();
- if (value->config == config && StringPiece(value->product) == product) {
+ if (value->config == config && value->product == product) {
return value;
}
}
@@ -185,10 +187,10 @@
ResourceConfigValue* ResourceEntry::FindOrCreateValue(const ConfigDescription& config,
StringPiece product) {
auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product},
- lt_config_key_ref<std::unique_ptr<ResourceConfigValue>>);
+ lt_config_key_ref());
if (iter != values.end()) {
ResourceConfigValue* value = iter->get();
- if (value->config == config && StringPiece(value->product) == product) {
+ if (value->config == config && value->product == product) {
return value;
}
}
@@ -199,36 +201,21 @@
std::vector<ResourceConfigValue*> ResourceEntry::FindAllValues(const ConfigDescription& config) {
std::vector<ResourceConfigValue*> results;
-
- auto iter = values.begin();
+ auto iter =
+ std::lower_bound(values.begin(), values.end(), ConfigKey{&config, ""}, lt_config_key_ref());
for (; iter != values.end(); ++iter) {
ResourceConfigValue* value = iter->get();
- if (value->config == config) {
- results.push_back(value);
- ++iter;
+ if (value->config != config) {
break;
}
- }
-
- for (; iter != values.end(); ++iter) {
- ResourceConfigValue* value = iter->get();
- if (value->config == config) {
- results.push_back(value);
- }
+ results.push_back(value);
}
return results;
}
bool ResourceEntry::HasDefaultValue() const {
- const ConfigDescription& default_config = ConfigDescription::DefaultConfig();
-
// The default config should be at the top of the list, since the list is sorted.
- for (auto& config_value : values) {
- if (config_value->config == default_config) {
- return true;
- }
- }
- return false;
+ return !values.empty() && values.front()->config == ConfigDescription::DefaultConfig();
}
ResourceTable::CollisionResult ResourceTable::ResolveFlagCollision(FlagStatus existing,
@@ -364,14 +351,14 @@
if (found) {
return &*it;
}
- return &*el.insert(it, std::forward<T>(value));
+ return &*el.insert(it, std::move(value));
}
};
struct PackageViewComparer {
bool operator()(const ResourceTablePackageView& lhs, const ResourceTablePackageView& rhs) {
return less_than_struct_with_name_and_id<ResourceTablePackageView, uint8_t>(
- lhs, std::make_pair(rhs.name, rhs.id));
+ lhs, std::tie(rhs.name, rhs.id));
}
};
@@ -384,7 +371,7 @@
struct EntryViewComparer {
bool operator()(const ResourceTableEntryView& lhs, const ResourceTableEntryView& rhs) {
return less_than_struct_with_name_and_id<ResourceTableEntryView, uint16_t>(
- lhs, std::make_pair(rhs.name, rhs.id));
+ lhs, std::tie(rhs.name, rhs.id));
}
};
@@ -429,10 +416,10 @@
const ResourceConfigValue* ResourceTableEntryView::FindValue(const ConfigDescription& config,
android::StringPiece product) const {
auto iter = std::lower_bound(values.begin(), values.end(), ConfigKey{&config, product},
- lt_config_key_ref<const ResourceConfigValue*>);
+ lt_config_key_ref());
if (iter != values.end()) {
const ResourceConfigValue* value = *iter;
- if (value->config == config && StringPiece(value->product) == product) {
+ if (value->config == config && value->product == product) {
return value;
}
}
@@ -615,11 +602,15 @@
result = ResolveValueCollision(config_value->value.get(), res.value.get());
}
switch (result) {
- case CollisionResult::kKeepBoth:
+ case CollisionResult::kKeepBoth: {
// Insert the value ignoring for duplicate configurations
- entry->values.push_back(util::make_unique<ResourceConfigValue>(res.config, res.product));
- entry->values.back()->value = std::move(res.value);
+ auto it = entry->values.insert(
+ std::lower_bound(entry->values.begin(), entry->values.end(),
+ ConfigKey{&res.config, res.product}, lt_config_key_ref()),
+ util::make_unique<ResourceConfigValue>(res.config, res.product));
+ (*it)->value = std::move(res.value);
break;
+ }
case CollisionResult::kTakeNew:
// Take the incoming value.