Skip to content

Commit

Permalink
Move cls_to_interdex_group state to ConfigFiles
Browse files Browse the repository at this point in the history 
Summary:
To infer the interdex grouping of our mergeables, we first need to build a mapping from class to its betamap group id for classes listed in the betamap. We call that `cls_to_interdex_group`. The existing handling of building such map uses static state and is really dated.

This change moves the ownership of `cls_to_interdex_group` to ConfigFiles. That gives us a global "singleton" state that's nicely encapsulated in ConfigFiles. So that Class Merging doesn't need to worry about its detail and can just acquire it when needed lazily.

Reviewed By: NTillmann

Differential Revision: D48812676

fbshipit-source-id: 58aa1bb6f2aa7b45cd1fbc69749e43597ebf8027
  • Loading branch information
@facebook-github-bot
Wei Zhang (Devinfra) authored and facebook-github-bot committed Sep 1, 2023
1 parent 3ad2b15 commit c0755f6
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 75 deletions.
35 changes: 35 additions & 0 deletions libredex/ConfigFiles.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ using namespace std::string_literals;

namespace {

constexpr const char* CLASS_MARKER_DELIMITER = "DexEndMarker";

class StringTabSplitter {
private:
std::string_view m_line;
Expand Down Expand Up @@ -528,3 +530,36 @@ void ConfigFiles::set_class_lists(
m_class_lists = std::move(l);
m_load_class_lists_attempted = true;
}

void ConfigFiles::build_cls_interdex_groups() {
const auto& interdex_order = get_coldstart_classes();
if (interdex_order.empty()) {
// No grouping based on interdex.
m_num_interdex_groups = 0;
return;
}

size_t group_id = 0;
for (auto it = interdex_order.begin(); it != interdex_order.end(); ++it) {
const auto& cls_name = *it;
bool is_marker_delim =
cls_name.find(CLASS_MARKER_DELIMITER) != std::string::npos;

if (is_marker_delim || std::next(it) == interdex_order.end()) {
group_id++;

if (is_marker_delim) {
continue;
}
}

DexType* type = DexType::get_type(cls_name);
if (type && m_cls_to_interdex_group.count(type) == 0) {
m_cls_to_interdex_group[type] = group_id;
}
}

// group_id + 1 represents the number of groups (considering the classes
// outside of the interdex order as a group on its own).
m_num_interdex_groups = group_id + 1;
}
20 changes: 20 additions & 0 deletions libredex/ConfigFiles.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <map>
#include <memory>
#include <string>
#include <unordered_map>
#include <unordered_set>
#include <vector>

Expand Down Expand Up @@ -173,6 +174,20 @@ struct ConfigFiles {

const api::AndroidSDK& get_android_sdk_api(int32_t min_sdk_api);

std::unordered_map<DexType*, size_t>& get_cls_interdex_groups() {
if (m_cls_to_interdex_group.empty()) {
build_cls_interdex_groups();
}
return m_cls_to_interdex_group;
}

size_t get_num_interdex_groups() {
if (m_cls_to_interdex_group.empty()) {
build_cls_interdex_groups();
}
return m_num_interdex_groups;
}

void parse_global_config();

/**
Expand Down Expand Up @@ -203,6 +218,7 @@ struct ConfigFiles {
void build_dead_class_and_live_class_split_lists();
bool is_relocated_class(std::string_view name) const;
void remove_relocated_part(std::string_view* name);
void build_cls_interdex_groups();

// For testing.
void set_class_lists(
Expand Down Expand Up @@ -240,6 +256,10 @@ struct ConfigFiles {
// min_sdk AndroidAPI
int32_t m_min_sdk_api_level = 0;
std::unique_ptr<api::AndroidSDK> m_android_min_sdk_api;
// interdex class group based on betamap
// 0 when no interdex grouping.
size_t m_num_interdex_groups = 0;
std::unordered_map<DexType*, size_t> m_cls_to_interdex_group;

friend struct ClassPreloadTest;
};
27 changes: 5 additions & 22 deletions service/class-merging/ClassMerging.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,12 @@ using namespace class_merging;

namespace {

bool s_is_initialized = false;

/*
* Initialize a number of static states for the output mapping file and the
* interdex group mapping.
*/
void set_up(ConfigFiles& conf) {
if (s_is_initialized) {
// Already initialized.
return;
}
Model::build_interdex_groups(conf);
s_is_initialized = true;
}

/**
* Create a ref checker for checking cross-store references and Android SDK api
* usages. When the per_dex_grouping is false, the create ref checker will check
* cross-store references. When the per_dex_grouping is true, the checker
* doesn't check cross-store reference and will let the class merging grouping
* do the correct grouping for each dex.
* Create a ref checker for checking cross-store references and Android SDK
* api usages. When the per_dex_grouping is false, the create ref checker will
* check cross-store references. When the per_dex_grouping is true, the
* checker doesn't check cross-store reference and will let the class merging
* grouping do the correct grouping for each dex.
*/
std::unique_ptr<RefChecker> create_ref_checker(const bool per_dex_grouping,
XStoreRefs* xstores,
Expand Down Expand Up @@ -136,8 +121,6 @@ ModelStats merge_model(const TypeSystem& type_system,
PassManager& mgr,
DexStoresVector& stores,
ModelSpec& spec) {
set_up(conf);
always_assert(s_is_initialized);
TRACE(CLMG,
2,
"[ClassMerging] merging %s model merging targets %zu roots %zu",
Expand Down
56 changes: 11 additions & 45 deletions service/class-merging/Model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,8 @@

using namespace class_merging;

size_t Model::s_num_interdex_groups = 0;
std::unordered_map<DexType*, size_t> Model::s_cls_to_interdex_group;

namespace {

constexpr const char* CLASS_MARKER_DELIMITER = "DexEndMarker";

std::string to_string(const ModelSpec& spec) {
std::ostringstream ss;
ss << spec.name << "(roots: ";
Expand Down Expand Up @@ -168,7 +163,7 @@ const TypeSet Model::empty_set = TypeSet();

Model::Model(const Scope& scope,
const DexStoresVector& stores,
const ConfigFiles& conf,
ConfigFiles& conf,
const ModelSpec& spec,
const TypeSystem& type_system,
const RefChecker& refchecker)
Expand Down Expand Up @@ -252,39 +247,6 @@ MergerType* Model::build_mergers(const DexType* root) {
return &merger;
}

void Model::build_interdex_groups(ConfigFiles& conf) {
const auto& interdex_order = conf.get_coldstart_classes();
if (interdex_order.empty()) {
// No grouping based on interdex.
s_num_interdex_groups = 0;
return;
}

size_t group_id = 0;
for (auto it = interdex_order.begin(); it != interdex_order.end(); ++it) {
const auto& cls_name = *it;
bool is_marker_delim =
cls_name.find(CLASS_MARKER_DELIMITER) != std::string::npos;

if (is_marker_delim || std::next(it) == interdex_order.end()) {
group_id++;

if (is_marker_delim) {
continue;
}
}

DexType* type = DexType::get_type(cls_name);
if (type && s_cls_to_interdex_group.count(type) == 0) {
s_cls_to_interdex_group[type] = group_id;
}
}

// group_id + 1 represents the number of groups (considering the classes
// outside of the interdex order as a group on its own).
s_num_interdex_groups = group_id + 1;
}

MergerType& Model::create_dummy_merger(const DexType* type) {
auto& merger = m_mergers[type];
merger.type = type;
Expand Down Expand Up @@ -837,9 +799,13 @@ TypeSet Model::get_types_in_current_interdex_group(
*/
std::vector<ConstTypeHashSet> Model::group_by_interdex_set(
const ConstTypeHashSet& types) {
const auto& cls_to_interdex_groups = m_conf.get_cls_interdex_groups();
auto num_interdex_groups = m_conf.get_num_interdex_groups();
TRACE(CLMG, 5, "num_interdex_groups %zu; cls_to_interdex_groups %zu",
num_interdex_groups, cls_to_interdex_groups.size());
size_t num_group = 1;
if (is_interdex_grouping_enabled() && s_num_interdex_groups > 1) {
num_group = s_num_interdex_groups;
if (is_interdex_grouping_enabled() && num_interdex_groups > 1) {
num_group = num_interdex_groups;
}
std::vector<ConstTypeHashSet> new_groups(num_group);
if (num_group == 1) {
Expand All @@ -849,16 +815,16 @@ std::vector<ConstTypeHashSet> Model::group_by_interdex_set(
const auto& type_to_usages =
get_type_usages(types, m_scope, m_spec.interdex_grouping_inferring_mode);
for (const auto& pair : type_to_usages) {
auto index = get_interdex_group(pair.second, s_cls_to_interdex_group,
s_num_interdex_groups);
auto index = get_interdex_group(pair.second, cls_to_interdex_groups,
num_interdex_groups);
if (m_spec.interdex_grouping == InterDexGroupingType::NON_HOT_SET) {
if (index == 0) {
// Drop mergeables that are in the hot set.
continue;
}
} else if (m_spec.interdex_grouping ==
InterDexGroupingType::NON_ORDERED_SET) {
if (index < s_num_interdex_groups - 1) {
if (index < num_interdex_groups - 1) {
// Only merge the last group which are not in ordered set, drop other
// mergeables.
continue;
Expand Down Expand Up @@ -1524,7 +1490,7 @@ std::string Model::print(const DexType* type, int nest) const {

Model Model::build_model(const Scope& scope,
const DexStoresVector& stores,
const ConfigFiles& conf,
ConfigFiles& conf,
const ModelSpec& spec,
const TypeSystem& type_system,
const RefChecker& refchecker) {
Expand Down
11 changes: 3 additions & 8 deletions service/class-merging/Model.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class Model {
*/
static Model build_model(const Scope& scope,
const DexStoresVector& stores,
const ConfigFiles& conf,
ConfigFiles& conf,
const ModelSpec& spec,
const TypeSystem& type_system,
const RefChecker& refchecker);
Expand Down Expand Up @@ -304,8 +304,6 @@ class Model {
bool process_method_meta() const { return m_spec.process_method_meta; }
bool keep_debug_info() const { return m_spec.keep_debug_info; }

static void build_interdex_groups(ConfigFiles& conf);

/**
* Print everything about the model.
* The printing has a format to allow grep to isolate specific parts.
Expand Down Expand Up @@ -383,19 +381,16 @@ class Model {
std::map<MergerType::Shape, size_t, MergerType::ShapeComp> m_shape_to_count;

const Scope& m_scope;
const ConfigFiles& m_conf;
ConfigFiles& m_conf;
const XDexRefs m_x_dex;

static std::unordered_map<DexType*, size_t> s_cls_to_interdex_group;
static size_t s_num_interdex_groups;

/**
* Build a Model given a set of roots and a set of types deriving from the
* roots.
*/
Model(const Scope& scope,
const DexStoresVector& stores,
const ConfigFiles& conf,
ConfigFiles& conf,
const ModelSpec& spec,
const TypeSystem& type_system,
const RefChecker& refchecker);
Expand Down

0 comments on commit c0755f6

Please sign in to comment.