Skip to content

Commit

Permalink
Back out "Back out "[redex] [RMU] run dce on methods affected by code…
Browse files Browse the repository at this point in the history
… changes""

Summary:
Original commit changeset: 1d2759209dcc

Original Phabricator Diff: D51860446

Reviewed By: ssj933

Differential Revision: D51873079

fbshipit-source-id: 1903515c5690c8e0bd29eccfa360381c388ef9fa
  • Loading branch information
Nikolai Tillmann authored and facebook-github-bot committed Dec 6, 2023
1 parent 702b4c3 commit 4a4543f
Show file tree
Hide file tree
Showing 9 changed files with 114 additions and 60 deletions.
16 changes: 5 additions & 11 deletions libredex/Reachability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1907,7 +1907,8 @@ void ReachableAspects::finish(const ConditionallyMarked& cond_marked,
}

std::unique_ptr<ReachableObjects> compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
const IgnoreSets& ignore_sets,
int* num_ignore_check_strings,
ReachableAspects* reachable_aspects,
Expand All @@ -1918,17 +1919,14 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects(
bool cfg_gathering_check_instance_callable,
bool cfg_gathering_check_returning,
bool should_mark_all_as_seed,
std::unique_ptr<const mog::Graph>* out_method_override_graph,
bool remove_no_argument_constructors) {
Timer t("Marking");
auto scope = build_class_scope(stores);
std::unordered_set<const DexClass*> scope_set(scope.begin(), scope.end());
auto reachable_objects = std::make_unique<ReachableObjects>();
ConditionallyMarked cond_marked;
auto method_override_graph = mog::build_graph(scope);

ConcurrentSet<ReachableObject, ReachableObjectHash> root_set;
RootSetMarker root_set_marker(*method_override_graph, record_reachability,
RootSetMarker root_set_marker(method_override_graph, record_reachability,
relaxed_keep_class_members,
remove_no_argument_constructors, &cond_marked,
reachable_objects.get(), &root_set);
Expand All @@ -1944,7 +1942,7 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects(
TransitiveClosureMarkerSharedState shared_state{
std::move(scope_set),
&ignore_sets,
method_override_graph.get(),
&method_override_graph,
record_reachability,
relaxed_keep_class_members,
relaxed_keep_interfaces,
Expand All @@ -1965,17 +1963,13 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects(
},
root_set, num_threads,
/*push_tasks_while_running=*/true);
compute_zombie_methods(*method_override_graph, *reachable_objects,
compute_zombie_methods(method_override_graph, *reachable_objects,
*reachable_aspects);

if (num_ignore_check_strings != nullptr) {
*num_ignore_check_strings = (int)stats.num_ignore_check_strings;
}

if (out_method_override_graph) {
*out_method_override_graph = std::move(method_override_graph);
}

reachable_aspects->finish(cond_marked, *reachable_objects);

return reachable_objects;
Expand Down
5 changes: 2 additions & 3 deletions libredex/Reachability.h
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,8 @@ class TransitiveClosureMarkerWorker {
* (e.g. proguard rules).
*/
std::unique_ptr<ReachableObjects> compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
const IgnoreSets& ignore_sets,
int* num_ignore_check_strings,
ReachableAspects* reachable_aspects,
Expand All @@ -702,8 +703,6 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects(
bool cfg_gathering_check_instance_callable = false,
bool cfg_gathering_check_returning = false,
bool should_mark_all_as_seed = false,
std::unique_ptr<const method_override_graph::Graph>*
out_method_override_graph = nullptr,
bool remove_no_argument_constructors = false);

void compute_zombie_methods(
Expand Down
52 changes: 42 additions & 10 deletions opt/remove-unreachable/RemoveUnreachable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "ConfigFiles.h"
#include "DexUtil.h"
#include "IOUtil.h"
#include "InitClassesWithSideEffects.h"
#include "LocalDce.h"
#include "MethodOverrideGraph.h"
#include "PassManager.h"
#include "Show.h"
Expand Down Expand Up @@ -176,6 +178,18 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
// Store names of removed classes and methods
ConcurrentSet<std::string> removed_symbols;

auto sweep_code = m_prune_uninstantiable_insns || m_throw_propagation;
auto scope = build_class_scope(stores);
always_assert(!pm.unreliable_virtual_scopes());
auto method_override_graph = mog::build_graph(scope);
std::unique_ptr<init_classes::InitClassesWithSideEffects>
init_classes_with_side_effects;
if (sweep_code && !pm.init_class_lowering_has_run()) {
init_classes_with_side_effects =
std::make_unique<init_classes::InitClassesWithSideEffects>(
scope, conf.create_init_class_insns(), method_override_graph.get());
}

root_metrics(stores, pm);

bool emit_graph_this_run =
Expand All @@ -193,8 +207,8 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
int num_ignore_check_strings = 0;
reachability::ReachableAspects reachable_aspects;
auto reachables = this->compute_reachable_objects(
stores, pm, &num_ignore_check_strings, &reachable_aspects,
emit_graph_this_run, m_relaxed_keep_class_members,
scope, *method_override_graph, pm, &num_ignore_check_strings,
&reachable_aspects, emit_graph_this_run, m_relaxed_keep_class_members,
m_prune_unreferenced_interfaces, m_prune_uninstantiable_insns,
m_prune_uncallable_instance_method_bodies, m_throw_propagation,
m_remove_no_argument_constructors);
Expand All @@ -219,7 +233,7 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
auto abstracted_classes = reachability::mark_classes_abstract(
stores, *reachables, reachable_aspects);
pm.incr_metric("abstracted_classes", abstracted_classes.size());
if (m_prune_uninstantiable_insns || m_throw_propagation) {
if (sweep_code) {
remove_uninstantiables_impl::Stats remove_uninstantiables_stats;
std::atomic<size_t> throws_inserted{0};
InsertOnlyConcurrentSet<DexMethod*> affected_methods;
Expand All @@ -230,6 +244,23 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
remove_uninstantiables_stats.report(pm);
pm.incr_metric("throws_inserted", (size_t)throws_inserted);
pm.incr_metric("methods_with_code_changes", affected_methods.size());
std::unordered_set<DexMethodRef*> pure_methods;
LocalDce::Stats dce_stats;
std::mutex dce_stats_mutex;
workqueue_run<DexMethod*>(
[&](DexMethod* method) {
LocalDce dce(init_classes_with_side_effects.get(), pure_methods);
dce.dce(method->get_code()->cfg(), /* normalize_new_instances */ true,
method->get_class());
auto local_stats = dce.get_stats();
std::lock_guard<std::mutex> lock(dce_stats_mutex);
dce_stats += local_stats;
},
affected_methods);
pm.incr_metric("instructions_eliminated_localdce_dead",
dce_stats.dead_instruction_count);
pm.incr_metric("instructions_eliminated_localdce_unreachable",
dce_stats.unreachable_instruction_count);
}
reachability::sweep(stores, *reachables,
output_unreachable_symbols ? &removed_symbols : nullptr,
Expand Down Expand Up @@ -268,7 +299,7 @@ void RemoveUnreachablePassBase::run_pass(DexStoresVector& stores,
{
std::ofstream os;
open_or_die(conf.metafile("method-override-graph"), &os);
auto method_override_graph = mog::build_graph(build_class_scope(stores));
method_override_graph = mog::build_graph(build_class_scope(stores));
method_override_graph->dump(os);
}
}
Expand Down Expand Up @@ -301,7 +332,8 @@ void RemoveUnreachablePassBase::write_out_removed_symbols(

std::unique_ptr<reachability::ReachableObjects>
RemoveUnreachablePass::compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
PassManager& /* pm */,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
Expand All @@ -313,11 +345,11 @@ RemoveUnreachablePass::compute_reachable_objects(
bool cfg_gathering_check_returning,
bool remove_no_argument_constructors) {
return reachability::compute_reachable_objects(
stores, m_ignore_sets, num_ignore_check_strings, reachable_aspects,
emit_graph_this_run, relaxed_keep_class_members, relaxed_keep_interfaces,
cfg_gathering_check_instantiable, cfg_gathering_check_instance_callable,
cfg_gathering_check_returning, false, nullptr,
remove_no_argument_constructors);
scope, method_override_graph, m_ignore_sets, num_ignore_check_strings,
reachable_aspects, emit_graph_this_run, relaxed_keep_class_members,
relaxed_keep_interfaces, cfg_gathering_check_instantiable,
cfg_gathering_check_instance_callable, cfg_gathering_check_returning,
false, remove_no_argument_constructors);
}

static RemoveUnreachablePass s_pass;
27 changes: 15 additions & 12 deletions opt/remove-unreachable/RemoveUnreachable.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,19 @@ class RemoveUnreachablePassBase : public Pass {
void run_pass(DexStoresVector&, ConfigFiles&, PassManager&) override;

virtual std::unique_ptr<reachability::ReachableObjects>
compute_reachable_objects(const DexStoresVector& stores,
PassManager& pm,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
bool emit_graph_this_run,
bool relaxed_keep_class_members,
bool relaxed_keep_interfaces,
bool cfg_gathering_check_instantiable,
bool cfg_gathering_check_instance_callable,
bool cfg_gathering_check_returning,
bool remove_no_argument_constructors) = 0;
compute_reachable_objects(
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
PassManager& pm,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
bool emit_graph_this_run,
bool relaxed_keep_class_members,
bool relaxed_keep_interfaces,
bool cfg_gathering_check_instantiable,
bool cfg_gathering_check_instance_callable,
bool cfg_gathering_check_returning,
bool remove_no_argument_constructors) = 0;

void write_out_removed_symbols(
const std::string& filepath,
Expand All @@ -101,7 +103,8 @@ class RemoveUnreachablePass : public RemoveUnreachablePassBase {
: RemoveUnreachablePassBase("RemoveUnreachablePass") {}

std::unique_ptr<reachability::ReachableObjects> compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
PassManager& pm,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
Expand Down
25 changes: 13 additions & 12 deletions opt/remove-unreachable/TypeAnalysisAwareRemoveUnreachable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,8 @@ class TypeAnalysisAwareClosureMarkerWorker final
};

std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
const IgnoreSets& ignore_sets,
int* num_ignore_check_strings,
ReachableAspects* reachable_aspects,
Expand All @@ -355,18 +356,16 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
int* num_unreachable_invokes,
int* num_null_invokes) {
Timer t("Marking");
auto scope = build_class_scope(stores);
std::unordered_set<const DexClass*> scope_set(scope.begin(), scope.end());
walk::parallel::code(scope, [](DexMethod*, IRCode& code) {
code.cfg().calculate_exit_block();
});
auto reachable_objects = std::make_unique<ReachableObjects>();
ConditionallyMarked cond_marked;
auto method_override_graph = mog::build_graph(scope);

ConcurrentSet<ReachableObject, ReachableObjectHash> root_set;
bool remove_no_argument_constructors = false;
RootSetMarker root_set_marker(*method_override_graph, record_reachability,
RootSetMarker root_set_marker(method_override_graph, record_reachability,
relaxed_keep_class_members,
remove_no_argument_constructors, &cond_marked,
reachable_objects.get(), &root_set);
Expand All @@ -375,7 +374,7 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
size_t num_threads = redex_parallel::default_num_threads();
Stats stats;
TypeAnalysisAwareClosureMarkerSharedState shared_state{
{std::move(scope_set), &ignore_sets, method_override_graph.get(),
{std::move(scope_set), &ignore_sets, &method_override_graph,
record_reachability, relaxed_keep_class_members, relaxed_keep_interfaces,
cfg_gathering_check_instantiable, cfg_gathering_check_instance_callable,
cfg_gathering_check_returning, &cond_marked, reachable_objects.get(),
Expand All @@ -392,7 +391,7 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(
},
root_set, num_threads,
/* push_tasks_while_running*/ true);
compute_zombie_methods(*method_override_graph, *reachable_objects,
compute_zombie_methods(method_override_graph, *reachable_objects,
*reachable_aspects);

if (num_ignore_check_strings != nullptr) {
Expand All @@ -417,7 +416,8 @@ std::unique_ptr<ReachableObjects> compute_reachable_objects_with_type_anaysis(

std::unique_ptr<reachability::ReachableObjects>
TypeAnalysisAwareRemoveUnreachablePass::compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
PassManager& pm,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
Expand All @@ -438,11 +438,12 @@ TypeAnalysisAwareRemoveUnreachablePass::compute_reachable_objects(
int num_unreachable_invokes;
int num_null_invokes;
auto res = compute_reachable_objects_with_type_anaysis(
stores, m_ignore_sets, num_ignore_check_strings, reachable_aspects,
emit_graph_this_run, relaxed_keep_class_members, relaxed_keep_interfaces,
cfg_gathering_check_instantiable, cfg_gathering_check_instance_callable,
cfg_gathering_check_returning, gta.get(), remove_no_argument_constructors,
&num_exact_resolved_callees, &num_unreachable_invokes, &num_null_invokes);
scope, method_override_graph, m_ignore_sets, num_ignore_check_strings,
reachable_aspects, emit_graph_this_run, relaxed_keep_class_members,
relaxed_keep_interfaces, cfg_gathering_check_instantiable,
cfg_gathering_check_instance_callable, cfg_gathering_check_returning,
gta.get(), remove_no_argument_constructors, &num_exact_resolved_callees,
&num_unreachable_invokes, &num_null_invokes);
pm.incr_metric("num_exact_resolved_callees", num_exact_resolved_callees);
pm.incr_metric("num_unreachable_invokes", num_unreachable_invokes);
pm.incr_metric("num_null_invokes", num_null_invokes);
Expand Down
3 changes: 2 additions & 1 deletion opt/remove-unreachable/TypeAnalysisAwareRemoveUnreachable.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ class TypeAnalysisAwareRemoveUnreachablePass
}

std::unique_ptr<reachability::ReachableObjects> compute_reachable_objects(
const DexStoresVector& stores,
const Scope& scope,
const method_override_graph::Graph& method_override_graph,
PassManager& pm,
int* num_ignore_check_strings,
reachability::ReachableAspects* reachable_aspects,
Expand Down
20 changes: 15 additions & 5 deletions test/integ/FlowSensitiveReachabilityTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ TEST_F(FlowSensitiveReachabilityTest,
reachability::ReachableAspects reachable_aspects;
auto scope = build_class_scope(stores);
walk::parallel::code(scope, [&](auto*, auto& code) { code.build_cfg(); });
auto method_override_graph = method_override_graph::build_graph(scope);

auto reachable_objects = reachability::compute_reachable_objects(
stores, ig_sets, &num_ignore_check_strings, &reachable_aspects, false,
scope, *method_override_graph, ig_sets, &num_ignore_check_strings,
&reachable_aspects, false,
/* relaxed_keep_class_members */ true,
/* relaxed_keep_interfaces */ false,
/* cfg_gathering_check_instantiable */ true);
Expand Down Expand Up @@ -112,9 +114,11 @@ TEST_F(FlowSensitiveReachabilityTest, cfg_gathering_check_instance_callable) {
reachability::ReachableAspects reachable_aspects;
auto scope = build_class_scope(stores);
walk::parallel::code(scope, [&](auto*, auto& code) { code.build_cfg(); });
auto method_override_graph = method_override_graph::build_graph(scope);

auto reachable_objects = reachability::compute_reachable_objects(
stores, ig_sets, &num_ignore_check_strings, &reachable_aspects, false,
scope, *method_override_graph, ig_sets, &num_ignore_check_strings,
&reachable_aspects, false,
/* relaxed_keep_class_members */ true,
/* relaxed_keep_interfaces */ false,
/* cfg_gathering_check_instantiable */ true,
Expand Down Expand Up @@ -195,9 +199,11 @@ TEST_F(FlowSensitiveReachabilityTest, sweep_uncallable_virtual_methods) {
reachability::ReachableAspects reachable_aspects;
auto scope = build_class_scope(stores);
walk::parallel::code(scope, [&](auto*, auto& code) { code.build_cfg(); });
auto method_override_graph = method_override_graph::build_graph(scope);

auto reachable_objects = reachability::compute_reachable_objects(
stores, ig_sets, &num_ignore_check_strings, &reachable_aspects, false,
scope, *method_override_graph, ig_sets, &num_ignore_check_strings,
&reachable_aspects, false,
/* relaxed_keep_class_members */ true,
/* relaxed_keep_interfaces */ false,
/* cfg_gathering_check_instantiable */ true,
Expand Down Expand Up @@ -288,9 +294,11 @@ TEST_F(FlowSensitiveReachabilityTest, abstract_overrides_non_abstract) {
reachability::ReachableAspects reachable_aspects;
auto scope = build_class_scope(stores);
walk::parallel::code(scope, [&](auto*, auto& code) { code.build_cfg(); });
auto method_override_graph = method_override_graph::build_graph(scope);

auto reachable_objects = reachability::compute_reachable_objects(
stores, ig_sets, &num_ignore_check_strings, &reachable_aspects, false,
scope, *method_override_graph, ig_sets, &num_ignore_check_strings,
&reachable_aspects, false,
/* relaxed_keep_class_members */ true,
/* relaxed_keep_interfaces */ false,
/* cfg_gathering_check_instantiable */ true);
Expand Down Expand Up @@ -333,9 +341,11 @@ TEST_F(FlowSensitiveReachabilityTest, throw_propagation) {
reachability::ReachableAspects reachable_aspects;
auto scope = build_class_scope(stores);
walk::parallel::code(scope, [&](auto*, auto& code) { code.build_cfg(); });
auto method_override_graph = method_override_graph::build_graph(scope);

auto reachable_objects = reachability::compute_reachable_objects(
stores, ig_sets, &num_ignore_check_strings, &reachable_aspects, false,
scope, *method_override_graph, ig_sets, &num_ignore_check_strings,
&reachable_aspects, false,
/* relaxed_keep_class_members */ true,
/* relaxed_keep_interfaces */ true,
/* cfg_gathering_check_instantiable */ true,
Expand Down
Loading

0 comments on commit 4a4543f

Please sign in to comment.