Skip to content

Commit 1790677

Browse files
wsanvillefacebook-github-bot
authored andcommitted
Use OSDCE service to clean up old array filling
Summary: Implements the follow-up improvement mentioned at D53505200. Reviewed By: NTillmann Differential Revision: D55265832 fbshipit-source-id: dcaa7d19da94d9f096436561ef2ae7e02baa492e
1 parent 0d96a06 commit 1790677

File tree

1 file changed

+32
-7
lines changed

1 file changed

+32
-7
lines changed

service/resources/RClass.cpp

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,17 @@
1818
#include "DexUtil.h"
1919
#include "EditableCfgAdapter.h"
2020
#include "GlobalConfig.h"
21+
#include "InitClassesWithSideEffects.h"
2122
#include "InitDeps.h"
2223
#include "LiveRange.h"
23-
#include "LocalDce.h"
24+
#include "LocalPointersAnalysis.h"
25+
#include "MethodOverrideGraph.h"
26+
#include "ObjectSensitiveDce.h"
2427
#include "RedexResources.h"
2528
#include "Resolver.h"
2629
#include "ScopedCFG.h"
2730
#include "Show.h"
31+
#include "SideEffectSummary.h"
2832
#include "Timer.h"
2933
#include "Trace.h"
3034

@@ -426,6 +430,28 @@ bool remap_array(const std::vector<uint32_t>& original_values,
426430
}
427431
return changed;
428432
}
433+
434+
void perform_dce(Scope& scope) {
435+
auto method_override_graph = method_override_graph::build_graph(scope);
436+
init_classes::InitClassesWithSideEffects init_classes_with_side_effects(
437+
scope, true, method_override_graph.get());
438+
// Assume no pure methods or summaries for external/framework code for
439+
// simplicity. OSDCE should make conservative assumptions in the face of this.
440+
std::unordered_set<DexMethodRef*> pure_methods;
441+
local_pointers::SummaryMap escape_summaries;
442+
side_effects::SummaryMap effect_summaries;
443+
ObjectSensitiveDce impl(scope,
444+
&init_classes_with_side_effects,
445+
pure_methods,
446+
*method_override_graph,
447+
0, /* should not be encountering virtual calls here */
448+
&escape_summaries,
449+
&effect_summaries);
450+
impl.dce();
451+
auto& stats = impl.get_stats();
452+
TRACE(OPTRES, 2, "Pruned %zu instruction(s); R class scope size = %zu",
453+
stats.removed_instructions, scope.size());
454+
}
429455
} // namespace
430456

431457
size_t RClassWriter::remap_resource_class_clinit(
@@ -515,13 +541,7 @@ size_t RClassWriter::remap_resource_class_clinit(
515541
}
516542
auto i2 = cfg::InstructionIterable(cfg);
517543
mutation.insert_before(i2.begin(), consts);
518-
519544
mutation.flush();
520-
// OSDCE has the capability to mop up array creation and fills that go nowhere
521-
// but as a simple cleanup effort (for now) run LocalDce to perform some
522-
// cleanup since the former is not easily runnable on per-method basis right
523-
// now.
524-
LocalDce(/* init_classes_with_side_effects */ nullptr, {}).dce(ir_code);
525545
return pending_new_values.size();
526546
}
527547

@@ -545,12 +565,17 @@ void RClassWriter::remap_resource_class_arrays(
545565
});
546566

547567
// Actually remap the values in arrays.
568+
Scope cleanup_scope;
548569
for (auto& [cls, field_values] : class_states) {
570+
cleanup_scope.emplace_back(cls);
549571
if (!field_values.empty()) {
550572
auto changes =
551573
remap_resource_class_clinit(cls, field_values, old_to_remapped_ids);
552574
TRACE(OPTRES, 2, "Updated %zu field(s) in %s clinit", changes, SHOW(cls));
553575
}
554576
}
577+
// Modifying array values will leave behind old array filling instructions.
578+
// Perform DCE to clean this up.
579+
perform_dce(cleanup_scope);
555580
}
556581
} // namespace resources

0 commit comments

Comments
 (0)