From b54ef10f55ab9f11fdc73b0436b708e518fb5029 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Fri, 21 Oct 2016 13:38:42 -0700 Subject: [PATCH] AAPT2: Ensure string pool ordering is compact Keep styledstrings at the beginning of the StringPool to reduce the padding in the StyledString array. Bug:32336940 Test: manual Change-Id: Iec820c21a54daac40ecc3b2f87517a0f1efc9d3d --- tools/aapt2/ResourceParser.cpp | 7 +- tools/aapt2/StringPool.h | 17 +- tools/aapt2/flatten/XmlFlattener.cpp | 4 +- tools/aapt2/proto/TableProtoDeserializer.cpp | 787 +++++++-------- .../aapt2/unflatten/BinaryResourceParser.cpp | 934 +++++++++--------- 5 files changed, 899 insertions(+), 850 deletions(-) diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 51aed135a39e1..7d50e1d388164 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -533,7 +533,8 @@ std::unique_ptr ResourceParser::parseXml(xml::XmlPullParser* parser, if (!styleString.spans.empty()) { // This can only be a StyledString. return util::make_unique(mTable->stringPool.makeRef( - styleString, StringPool::Context{1, mConfig})); + styleString, + StringPool::Context(StringPool::Context::kStylePriority, mConfig))); } auto onCreateReference = [&](const ResourceName& name) { @@ -559,13 +560,13 @@ std::unique_ptr ResourceParser::parseXml(xml::XmlPullParser* parser, if (typeMask & android::ResTable_map::TYPE_STRING) { // Use the trimmed, escaped string. return util::make_unique(mTable->stringPool.makeRef( - styleString.str, StringPool::Context{1, mConfig})); + styleString.str, StringPool::Context(mConfig))); } if (allowRawValue) { // We can't parse this so return a RawString if we are allowed. return util::make_unique( - mTable->stringPool.makeRef(rawValue, StringPool::Context{1, mConfig})); + mTable->stringPool.makeRef(rawValue, StringPool::Context(mConfig))); } return {}; } diff --git a/tools/aapt2/StringPool.h b/tools/aapt2/StringPool.h index 05c26e7e4ea1f..6e0d646cae125 100644 --- a/tools/aapt2/StringPool.h +++ b/tools/aapt2/StringPool.h @@ -42,9 +42,22 @@ struct StyleString { class StringPool { public: - struct Context { - uint32_t priority; + class Context { + public: + enum : uint32_t { + kStylePriority = 0u, + kHighPriority = 1u, + kNormalPriority = 0x7fffffffu, + kLowPriority = 0xffffffffu, + }; + uint32_t priority = kNormalPriority; ConfigDescription config; + + Context() = default; + Context(uint32_t p, const ConfigDescription& c) : priority(p), config(c) {} + explicit Context(uint32_t p) : priority(p) {} + explicit Context(const ConfigDescription& c) + : priority(kNormalPriority), config(c) {} }; class Entry; diff --git a/tools/aapt2/flatten/XmlFlattener.cpp b/tools/aapt2/flatten/XmlFlattener.cpp index c296dde0ca1ec..b1536d5f21d2f 100644 --- a/tools/aapt2/flatten/XmlFlattener.cpp +++ b/tools/aapt2/flatten/XmlFlattener.cpp @@ -63,7 +63,7 @@ struct XmlFlattenerVisitor : public xml::Visitor { dest->index = util::deviceToHost32(-1); } else { mStringRefs.push_back(StringFlattenDest{ - mPool.makeRef(str, StringPool::Context{priority}), dest}); + mPool.makeRef(str, StringPool::Context(priority)), dest}); } } @@ -256,7 +256,7 @@ struct XmlFlattenerVisitor : public xml::Visitor { StringPool::Ref nameRef = mPackagePools[aaptAttr.id.value().packageId()].makeRef( - xmlAttr->name, StringPool::Context{aaptAttr.id.value().id}); + xmlAttr->name, StringPool::Context(aaptAttr.id.value().id)); // Add it to the list of strings to flatten. addString(nameRef, &flatAttr->name); diff --git a/tools/aapt2/proto/TableProtoDeserializer.cpp b/tools/aapt2/proto/TableProtoDeserializer.cpp index 595fa6f29faca..0dfb01c181c68 100644 --- a/tools/aapt2/proto/TableProtoDeserializer.cpp +++ b/tools/aapt2/proto/TableProtoDeserializer.cpp @@ -27,445 +27,460 @@ namespace aapt { namespace { class ReferenceIdToNameVisitor : public ValueVisitor { -public: - using ValueVisitor::visit; + public: + using ValueVisitor::visit; - explicit ReferenceIdToNameVisitor(const std::map* mapping) : - mMapping(mapping) { - assert(mMapping); + explicit ReferenceIdToNameVisitor( + const std::map* mapping) + : mMapping(mapping) { + assert(mMapping); + } + + void visit(Reference* reference) override { + if (!reference->id || !reference->id.value().isValid()) { + return; } - void visit(Reference* reference) override { - if (!reference->id || !reference->id.value().isValid()) { - return; - } - - ResourceId id = reference->id.value(); - auto cacheIter = mMapping->find(id); - if (cacheIter != mMapping->end()) { - reference->name = cacheIter->second.toResourceName(); - } + ResourceId id = reference->id.value(); + auto cacheIter = mMapping->find(id); + if (cacheIter != mMapping->end()) { + reference->name = cacheIter->second.toResourceName(); } + } -private: - const std::map* mMapping; + private: + const std::map* mMapping; }; class PackagePbDeserializer { -public: - PackagePbDeserializer(const android::ResStringPool* valuePool, - const android::ResStringPool* sourcePool, - const android::ResStringPool* symbolPool, - const Source& source, IDiagnostics* diag) : - mValuePool(valuePool), mSourcePool(sourcePool), mSymbolPool(symbolPool), - mSource(source), mDiag(diag) { + public: + PackagePbDeserializer(const android::ResStringPool* valuePool, + const android::ResStringPool* sourcePool, + const android::ResStringPool* symbolPool, + const Source& source, IDiagnostics* diag) + : mValuePool(valuePool), + mSourcePool(sourcePool), + mSymbolPool(symbolPool), + mSource(source), + mDiag(diag) {} + + public: + bool deserializeFromPb(const pb::Package& pbPackage, ResourceTable* table) { + Maybe id; + if (pbPackage.has_package_id()) { + id = static_cast(pbPackage.package_id()); } -public: - bool deserializeFromPb(const pb::Package& pbPackage, ResourceTable* table) { - Maybe id; - if (pbPackage.has_package_id()) { - id = static_cast(pbPackage.package_id()); - } + std::map idIndex; - std::map idIndex; + ResourceTablePackage* pkg = + table->createPackage(pbPackage.package_name(), id); + for (const pb::Type& pbType : pbPackage.types()) { + const ResourceType* resType = parseResourceType(pbType.name()); + if (!resType) { + mDiag->error(DiagMessage(mSource) << "unknown type '" << pbType.name() + << "'"); + return {}; + } - ResourceTablePackage* pkg = table->createPackage(pbPackage.package_name(), id); - for (const pb::Type& pbType : pbPackage.types()) { - const ResourceType* resType = parseResourceType(pbType.name()); - if (!resType) { - mDiag->error(DiagMessage(mSource) << "unknown type '" << pbType.name() << "'"); - return {}; + ResourceTableType* type = pkg->findOrCreateType(*resType); + + for (const pb::Entry& pbEntry : pbType.entries()) { + ResourceEntry* entry = type->findOrCreateEntry(pbEntry.name()); + + // Deserialize the symbol status (public/private with source and + // comments). + if (pbEntry.has_symbol_status()) { + const pb::SymbolStatus& pbStatus = pbEntry.symbol_status(); + if (pbStatus.has_source()) { + deserializeSourceFromPb(pbStatus.source(), *mSourcePool, + &entry->symbolStatus.source); + } + + if (pbStatus.has_comment()) { + entry->symbolStatus.comment = pbStatus.comment(); + } + + SymbolState visibility = + deserializeVisibilityFromPb(pbStatus.visibility()); + entry->symbolStatus.state = visibility; + + if (visibility == SymbolState::kPublic) { + // This is a public symbol, we must encode the ID now if there is + // one. + if (pbEntry.has_id()) { + entry->id = static_cast(pbEntry.id()); } - ResourceTableType* type = pkg->findOrCreateType(*resType); - - for (const pb::Entry& pbEntry : pbType.entries()) { - ResourceEntry* entry = type->findOrCreateEntry(pbEntry.name()); - - // Deserialize the symbol status (public/private with source and comments). - if (pbEntry.has_symbol_status()) { - const pb::SymbolStatus& pbStatus = pbEntry.symbol_status(); - if (pbStatus.has_source()) { - deserializeSourceFromPb(pbStatus.source(), *mSourcePool, - &entry->symbolStatus.source); - } - - if (pbStatus.has_comment()) { - entry->symbolStatus.comment = pbStatus.comment(); - } - - SymbolState visibility = deserializeVisibilityFromPb(pbStatus.visibility()); - entry->symbolStatus.state = visibility; - - if (visibility == SymbolState::kPublic) { - // This is a public symbol, we must encode the ID now if there is one. - if (pbEntry.has_id()) { - entry->id = static_cast(pbEntry.id()); - } - - if (type->symbolStatus.state != SymbolState::kPublic) { - // If the type has not been made public, do so now. - type->symbolStatus.state = SymbolState::kPublic; - if (pbType.has_id()) { - type->id = static_cast(pbType.id()); - } - } - } else if (visibility == SymbolState::kPrivate) { - if (type->symbolStatus.state == SymbolState::kUndefined) { - type->symbolStatus.state = SymbolState::kPrivate; - } - } - } - - ResourceId resId(pbPackage.package_id(), pbType.id(), pbEntry.id()); - if (resId.isValid()) { - idIndex[resId] = ResourceNameRef(pkg->name, type->type, entry->name); - } - - for (const pb::ConfigValue& pbConfigValue : pbEntry.config_values()) { - const pb::ConfigDescription& pbConfig = pbConfigValue.config(); - - ConfigDescription config; - if (!deserializeConfigDescriptionFromPb(pbConfig, &config)) { - mDiag->error(DiagMessage(mSource) << "invalid configuration"); - return {}; - } - - ResourceConfigValue* configValue = entry->findOrCreateValue(config, - pbConfig.product()); - if (configValue->value) { - // Duplicate config. - mDiag->error(DiagMessage(mSource) << "duplicate configuration"); - return {}; - } - - configValue->value = deserializeValueFromPb(pbConfigValue.value(), - config, &table->stringPool); - if (!configValue->value) { - return {}; - } - } + if (type->symbolStatus.state != SymbolState::kPublic) { + // If the type has not been made public, do so now. + type->symbolStatus.state = SymbolState::kPublic; + if (pbType.has_id()) { + type->id = static_cast(pbType.id()); + } } + } else if (visibility == SymbolState::kPrivate) { + if (type->symbolStatus.state == SymbolState::kUndefined) { + type->symbolStatus.state = SymbolState::kPrivate; + } + } } - ReferenceIdToNameVisitor visitor(&idIndex); - visitAllValuesInPackage(pkg, &visitor); - return true; + ResourceId resId(pbPackage.package_id(), pbType.id(), pbEntry.id()); + if (resId.isValid()) { + idIndex[resId] = ResourceNameRef(pkg->name, type->type, entry->name); + } + + for (const pb::ConfigValue& pbConfigValue : pbEntry.config_values()) { + const pb::ConfigDescription& pbConfig = pbConfigValue.config(); + + ConfigDescription config; + if (!deserializeConfigDescriptionFromPb(pbConfig, &config)) { + mDiag->error(DiagMessage(mSource) << "invalid configuration"); + return {}; + } + + ResourceConfigValue* configValue = + entry->findOrCreateValue(config, pbConfig.product()); + if (configValue->value) { + // Duplicate config. + mDiag->error(DiagMessage(mSource) << "duplicate configuration"); + return {}; + } + + configValue->value = deserializeValueFromPb( + pbConfigValue.value(), config, &table->stringPool); + if (!configValue->value) { + return {}; + } + } + } } -private: - std::unique_ptr deserializeItemFromPb(const pb::Item& pbItem, + ReferenceIdToNameVisitor visitor(&idIndex); + visitAllValuesInPackage(pkg, &visitor); + return true; + } + + private: + std::unique_ptr deserializeItemFromPb(const pb::Item& pbItem, + const ConfigDescription& config, + StringPool* pool) { + if (pbItem.has_ref()) { + const pb::Reference& pbRef = pbItem.ref(); + std::unique_ptr ref = util::make_unique(); + if (!deserializeReferenceFromPb(pbRef, ref.get())) { + return {}; + } + return std::move(ref); + + } else if (pbItem.has_prim()) { + const pb::Primitive& pbPrim = pbItem.prim(); + android::Res_value prim = {}; + prim.dataType = static_cast(pbPrim.type()); + prim.data = pbPrim.data(); + return util::make_unique(prim); + + } else if (pbItem.has_id()) { + return util::make_unique(); + + } else if (pbItem.has_str()) { + const uint32_t idx = pbItem.str().idx(); + const std::string str = util::getString(*mValuePool, idx); + + const android::ResStringPool_span* spans = mValuePool->styleAt(idx); + if (spans && spans->name.index != android::ResStringPool_span::END) { + StyleString styleStr = {str}; + while (spans->name.index != android::ResStringPool_span::END) { + styleStr.spans.push_back( + Span{util::getString(*mValuePool, spans->name.index), + spans->firstChar, spans->lastChar}); + spans++; + } + return util::make_unique(pool->makeRef( + styleStr, + StringPool::Context(StringPool::Context::kStylePriority, config))); + } + return util::make_unique( + pool->makeRef(str, StringPool::Context(config))); + + } else if (pbItem.has_raw_str()) { + const uint32_t idx = pbItem.raw_str().idx(); + const std::string str = util::getString(*mValuePool, idx); + return util::make_unique( + pool->makeRef(str, StringPool::Context(config))); + + } else if (pbItem.has_file()) { + const uint32_t idx = pbItem.file().path_idx(); + const std::string str = util::getString(*mValuePool, idx); + return util::make_unique(pool->makeRef( + str, + StringPool::Context(StringPool::Context::kHighPriority, config))); + + } else { + mDiag->error(DiagMessage(mSource) << "unknown item"); + } + return {}; + } + + std::unique_ptr deserializeValueFromPb(const pb::Value& pbValue, const ConfigDescription& config, StringPool* pool) { - if (pbItem.has_ref()) { - const pb::Reference& pbRef = pbItem.ref(); - std::unique_ptr ref = util::make_unique(); - if (!deserializeReferenceFromPb(pbRef, ref.get())) { - return {}; - } - return std::move(ref); + const bool isWeak = pbValue.has_weak() ? pbValue.weak() : false; - } else if (pbItem.has_prim()) { - const pb::Primitive& pbPrim = pbItem.prim(); - android::Res_value prim = {}; - prim.dataType = static_cast(pbPrim.type()); - prim.data = pbPrim.data(); - return util::make_unique(prim); - - } else if (pbItem.has_id()) { - return util::make_unique(); - - } else if (pbItem.has_str()) { - const uint32_t idx = pbItem.str().idx(); - const std::string str = util::getString(*mValuePool, idx); - - const android::ResStringPool_span* spans = mValuePool->styleAt(idx); - if (spans && spans->name.index != android::ResStringPool_span::END) { - StyleString styleStr = { str }; - while (spans->name.index != android::ResStringPool_span::END) { - styleStr.spans.push_back(Span{ - util::getString(*mValuePool, spans->name.index), - spans->firstChar, - spans->lastChar - }); - spans++; - } - return util::make_unique( - pool->makeRef(styleStr, StringPool::Context{ 1, config })); - } - return util::make_unique( - pool->makeRef(str, StringPool::Context{ 1, config })); - - } else if (pbItem.has_raw_str()) { - const uint32_t idx = pbItem.raw_str().idx(); - const std::string str = util::getString(*mValuePool, idx); - return util::make_unique( - pool->makeRef(str, StringPool::Context{ 1, config })); - - } else if (pbItem.has_file()) { - const uint32_t idx = pbItem.file().path_idx(); - const std::string str = util::getString(*mValuePool, idx); - return util::make_unique( - pool->makeRef(str, StringPool::Context{ 0, config })); - - } else { - mDiag->error(DiagMessage(mSource) << "unknown item"); - } + std::unique_ptr value; + if (pbValue.has_item()) { + value = deserializeItemFromPb(pbValue.item(), config, pool); + if (!value) { return {}; - } + } - std::unique_ptr deserializeValueFromPb(const pb::Value& pbValue, - const ConfigDescription& config, - StringPool* pool) { - const bool isWeak = pbValue.has_weak() ? pbValue.weak() : false; - - std::unique_ptr value; - if (pbValue.has_item()) { - value = deserializeItemFromPb(pbValue.item(), config, pool); - if (!value) { - return {}; - } - - } else if (pbValue.has_compound_value()) { - const pb::CompoundValue& pbCompoundValue = pbValue.compound_value(); - if (pbCompoundValue.has_attr()) { - const pb::Attribute& pbAttr = pbCompoundValue.attr(); - std::unique_ptr attr = util::make_unique(isWeak); - attr->typeMask = pbAttr.format_flags(); - attr->minInt = pbAttr.min_int(); - attr->maxInt = pbAttr.max_int(); - for (const pb::Attribute_Symbol& pbSymbol : pbAttr.symbols()) { - Attribute::Symbol symbol; - deserializeItemCommon(pbSymbol, &symbol.symbol); - if (!deserializeReferenceFromPb(pbSymbol.name(), &symbol.symbol)) { - return {}; - } - symbol.value = pbSymbol.value(); - attr->symbols.push_back(std::move(symbol)); - } - value = std::move(attr); - - } else if (pbCompoundValue.has_style()) { - const pb::Style& pbStyle = pbCompoundValue.style(); - std::unique_ptr