Skip to content

Commit

Permalink
Exclude not cold constructor
Browse files Browse the repository at this point in the history
Summary: We want to avoid unfinalize and add write barrier to constructors that are not cold. With the hypothesis that write barrier might cause performance degrade.

Reviewed By: NTillmann

Differential Revision: D69432559

fbshipit-source-id: 1151b91d4665946f9e7155239de06b6d804c38a2
  • Loading branch information
ssj933 authored and facebook-github-bot committed Feb 24, 2025
1 parent 23a3c10 commit d9ed2fb
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 1 deletion.
17 changes: 17 additions & 0 deletions libredex/ConfigFiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,13 +479,15 @@ void ConfigFiles::load_inliner_config(inliner::InlinerConfig* inliner_config) {
fprintf(stderr, "WARNING: No inliner config\n");
return;
}
std::string unfinalize_perf_mode_str;
JsonWrapper jw(config);
jw.get("delete_non_virtuals", true, inliner_config->delete_non_virtuals);
jw.get("virtual", true, inliner_config->virtual_inline);
jw.get("true_virtual_inline", false, inliner_config->true_virtual_inline);
jw.get("relaxed_init_inline", false, inliner_config->relaxed_init_inline);
jw.get("unfinalize_relaxed_init_inline", false,
inliner_config->unfinalize_relaxed_init_inline);
jw.get("unfinalize_perf_mode", "not-cold", unfinalize_perf_mode_str);
jw.get("strict_throwable_init_inline", false,
inliner_config->strict_throwable_init_inline);
jw.get("throws", false, inliner_config->throws_inline);
Expand Down Expand Up @@ -549,6 +551,21 @@ void ConfigFiles::load_inliner_config(inliner::InlinerConfig* inliner_config) {
type_s.c_str());
}
}

const static std::unordered_map<std::string, inliner::UnfinalizePerfMode>
unfinalize_perf_mode_mapping = {
{"none", inliner::UnfinalizePerfMode::NONE},
{"not-cold", inliner::UnfinalizePerfMode::NOT_COLD},
{"maybe-hot", inliner::UnfinalizePerfMode::MAYBE_HOT},
{"hot", inliner::UnfinalizePerfMode::HOT}};
always_assert_log(
unfinalize_perf_mode_mapping.count(unfinalize_perf_mode_str) > 0,
"Unexpected unfinalize perf mode input provided %s",
unfinalize_perf_mode_str.c_str());
inliner_config->unfinalize_perf_mode =
unfinalize_perf_mode_mapping.at(unfinalize_perf_mode_str);
inliner_config->unfinalize_perf_mode_str =
std::move(unfinalize_perf_mode_str);
}

const inliner::InlinerConfig& ConfigFiles::get_inliner_config() {
Expand Down
2 changes: 2 additions & 0 deletions libredex/GlobalConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ void InlinerConfig::bind_config() {
bind("relaxed_init_inline", relaxed_init_inline, relaxed_init_inline);
bind("unfinalize_relaxed_init_inline", unfinalize_relaxed_init_inline,
unfinalize_relaxed_init_inline);
bind("unfinalize_perf_mode", "not-cold", unfinalize_perf_mode_str,
"one of \"none\", \"not-cold\", \"maybe-hot\", \"hot\"");
bind("strict_throwable_init_inline", strict_throwable_init_inline,
strict_throwable_init_inline);
bind("intermediate_shrinking", intermediate_shrinking,
Expand Down
10 changes: 10 additions & 0 deletions libredex/InlinerConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ const size_t MAX_COST_FOR_CONSTANT_PROPAGATION = 5000;

namespace inliner {

enum UnfinalizePerfMode {
NONE = 0,
NOT_COLD = 1,
MAYBE_HOT = 2,
HOT = 3,
};

/**
* The global inliner config.
*/
Expand Down Expand Up @@ -60,6 +67,9 @@ struct InlinerConfig {
// analysis redex compiler can tolerate when making decision to inline
size_t max_cost_for_constant_propagation{MAX_COST_FOR_CONSTANT_PROPAGATION};

std::string unfinalize_perf_mode_str{"not-cold"};
UnfinalizePerfMode unfinalize_perf_mode{UnfinalizePerfMode::NOT_COLD};

// We will populate the information to rstate of classes and methods.
std::unordered_set<DexType*> no_inline_annos;
std::unordered_set<DexType*> force_inline_annos;
Expand Down
40 changes: 39 additions & 1 deletion service/method-inliner/MethodInliner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "ScopedCFG.h"
#include "ScopedMetrics.h"
#include "Shrinker.h"
#include "SourceBlocks.h"
#include "StlUtil.h"
#include "Timer.h"
#include "Walkers.h"
Expand Down Expand Up @@ -719,6 +720,24 @@ void gather_true_virtual_methods(
}
}

// If a method's entry block is hot, consider this method is hot.
bool is_hot(DexMethod* method) {
auto& cfg = method->get_code()->cfg();
return source_blocks::is_hot(cfg.entry_block());
}

// If a method's entry block may be hot, consider this method may be hot.
bool maybe_hot(DexMethod* method) {
auto& cfg = method->get_code()->cfg();
return source_blocks::maybe_hot(cfg.entry_block());
}

// If a method's entry block is not cold, consider this method is not cold.
bool is_not_cold(DexMethod* method) {
auto& cfg = method->get_code()->cfg();
return source_blocks::is_not_cold(cfg.entry_block());
}

// If a constructor cannot be inlined because it writes to final fields, then we
// can "simply" remove the final modifier. We typically run the
// AccessMarkingMarking pass multiple times, including after the final inliner,
Expand All @@ -731,6 +750,7 @@ void gather_true_virtual_methods(
// inserting a write barrier pseudo instruction to lower at the end.
void unfinalize_fields_if_beneficial_for_relaxed_init_inlining(
const Scope& scope,
const inliner::UnfinalizePerfMode& unfinalize_perf_mode,
InsertOnlyConcurrentMap<DexField*, std::vector<DexMethod*>>*
unfinalized_fields) {
walk::parallel::classes(scope, [&](DexClass* cls) {
Expand All @@ -754,7 +774,17 @@ void unfinalize_fields_if_beneficial_for_relaxed_init_inlining(
}
std::unordered_map<DexField*, std::vector<DexMethod*>>
local_unfinalized_fields;
bool perf_sensitive = false;
for (auto* init_method : cls->get_ctors()) {
if ((unfinalize_perf_mode == inliner::UnfinalizePerfMode::NOT_COLD &&
is_not_cold(init_method)) ||
(unfinalize_perf_mode == inliner::UnfinalizePerfMode::MAYBE_HOT &&
maybe_hot(init_method)) ||
(unfinalize_perf_mode == inliner::UnfinalizePerfMode::HOT &&
is_hot(init_method))) {
perf_sensitive = true;
break;
}
std::unordered_set<DexField*> written_final_fields;
if (!constructor_analysis::can_inline_init(
init_method, /* finalizable_fields */ nullptr,
Expand All @@ -765,6 +795,14 @@ void unfinalize_fields_if_beneficial_for_relaxed_init_inlining(
}
}
}
if (perf_sensitive) {
// If we turned on perf checking mode and any constructor meet the check,
// we need to give up unfinalizing the field, otherwise even not inlining
// the non-hot constructor, we would still need to add write barrier
// before return if there is another constructor being inlined and the
// field kept being unfinalized.
return;
}
for (auto&& [field, init_methods] : local_unfinalized_fields) {
field->set_access(field->get_access() & ~ACC_FINAL);
auto emplaced =
Expand Down Expand Up @@ -872,7 +910,7 @@ void run_inliner(
if (inliner_config.relaxed_init_inline &&
inliner_config.unfinalize_relaxed_init_inline && min_sdk >= 21) {
unfinalize_fields_if_beneficial_for_relaxed_init_inlining(
scope, &unfinalized_fields);
scope, inliner_config.unfinalize_perf_mode, &unfinalized_fields);
mgr.set_metric("unfinalized_fields", unfinalized_fields.size());
TRACE(INLINE, 3, "unfinalized %zu fields", unfinalized_fields.size());
for (auto&& [_, init_methods] : unfinalized_fields) {
Expand Down
1 change: 1 addition & 0 deletions test/instr/MethodInlineUnfinalizedRelaxedInit.config
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"inliner": {
"relaxed_init_inline": true,
"unfinalize_relaxed_init_inline": true,
"unfinalize_perf_mode": "hot",
"no_inline_annos" : [
"Lcom/fasterxml/jackson/databind/annotation/JsonDeserialize;"
],
Expand Down

0 comments on commit d9ed2fb

Please sign in to comment.