Skip to content

Commit

Permalink
don't use RedexContext
Browse files Browse the repository at this point in the history
Summary: Directly reaching into the RedexContext is not a great design. This is getting refactored here. It's also faster now due to parallelization.

Reviewed By: beicy

Differential Revision: D49083252

fbshipit-source-id: 2c7d4af27b07d9e50d48f358a42073475bfa15ef
  • Loading branch information
Nikolai Tillmann authored and facebook-github-bot committed Sep 15, 2023
1 parent 1ba46cb commit d8f2552
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 39 deletions.
30 changes: 14 additions & 16 deletions libredex/HierarchyUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,28 @@
#include "HierarchyUtil.h"

#include "RedexContext.h"
#include "Walkers.h"

namespace mog = method_override_graph;

namespace hierarchy_util {

std::unordered_set<const DexMethod*> find_non_overridden_virtuals(
const mog::Graph& override_graph) {
std::unordered_set<const DexMethod*> non_overridden_virtuals;
g_redex->walk_type_class([&](const DexType*, const DexClass* cls) {
if (!cls->is_external()) {
for (auto* method : cls->get_vmethods()) {
if (!mog::any_overriding_methods(override_graph, method)) {
non_overridden_virtuals.emplace(method);
}
}
} else {
for (auto* method : cls->get_vmethods()) {
if (is_final(cls) || is_final(method)) {
non_overridden_virtuals.emplace(method);
}
NonOverriddenVirtuals::NonOverriddenVirtuals(
const Scope& scope, const method_override_graph::Graph& override_graph) {
walk::parallel::classes(scope, [&](DexClass* cls) {
for (auto* method : cls->get_vmethods()) {
if (!mog::any_overriding_methods(override_graph, method)) {
m_non_overridden_virtuals.emplace(method);
}
}
});
return non_overridden_virtuals;
}

size_t NonOverriddenVirtuals::count(const DexMethod* method) const {
if (method->is_external()) {
return is_final(method) || is_final(type_class(method->get_class()));
}
return m_non_overridden_virtuals.count_unsafe(method);
}

} // namespace hierarchy_util
28 changes: 14 additions & 14 deletions libredex/HierarchyUtil.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,29 @@

#include <unordered_set>

#include "ConcurrentContainers.h"
#include "DexClass.h"
#include "MethodOverrideGraph.h"

namespace hierarchy_util {

/*
* Returns all non-overridden virtual methods in scope, plus methods from
* Identifies all non-overridden virtual methods in scope, plus methods from
* external classes. The external classes will be included even if they are not
* in the input Scope parameter.
*/
std::unordered_set<const DexMethod*> find_non_overridden_virtuals(
const method_override_graph::Graph& override_graph);
class NonOverriddenVirtuals {
public:
NonOverriddenVirtuals(const Scope& scope,
const method_override_graph::Graph& override_graph);
explicit NonOverriddenVirtuals(const Scope& scope)
: NonOverriddenVirtuals(scope,
*method_override_graph::build_graph(scope)) {}

/*
* Returns all non-overridden virtual methods in scope, plus methods from
* external classes. The external classes will be included even if they are not
* in the input Scope parameter.
*/
inline std::unordered_set<const DexMethod*> find_non_overridden_virtuals(
const Scope& scope) {
std::unique_ptr<const method_override_graph::Graph> override_graph =
method_override_graph::build_graph(scope);
return find_non_overridden_virtuals(*override_graph);
}
size_t count(const DexMethod*) const;

private:
ConcurrentSet<const DexMethod*> m_non_overridden_virtuals;
};

} // namespace hierarchy_util
8 changes: 4 additions & 4 deletions opt/object-escape-analysis/ObjectEscapeAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ using Locations = std::vector<std::pair<DexMethod*, const IRInstruction*>>;
// invocation dependencies.
void analyze_scope(
const Scope& scope,
const std::unordered_set<const DexMethod*>& non_overridden_virtuals,
const hier::NonOverriddenVirtuals& non_overridden_virtuals,
std::unordered_map<DexType*, Locations>* new_instances,
std::unordered_map<DexMethod*, Locations>* invokes,
std::unordered_map<DexMethod*, std::unordered_set<DexMethod*>>*
Expand Down Expand Up @@ -366,7 +366,7 @@ MethodSummaries compute_method_summaries(
const Scope& scope,
const std::unordered_map<DexMethod*, std::unordered_set<DexMethod*>>&
dependencies,
const std::unordered_set<const DexMethod*>& non_overridden_virtuals) {
const hier::NonOverriddenVirtuals& non_overridden_virtuals) {
Timer t("compute_method_summaries");

std::unordered_set<DexMethod*> impacted_methods;
Expand Down Expand Up @@ -1704,8 +1704,8 @@ void ObjectEscapeAnalysisPass::run_pass(DexStoresVector& stores,
auto method_override_graph = mog::build_graph(scope);
init_classes::InitClassesWithSideEffects init_classes_with_side_effects(
scope, conf.create_init_class_insns(), method_override_graph.get());
auto non_overridden_virtuals =
hier::find_non_overridden_virtuals(*method_override_graph);
hier::NonOverriddenVirtuals non_overridden_virtuals(scope,
*method_override_graph);

std::unordered_map<DexType*, Locations> new_instances;
std::unordered_map<DexMethod*, Locations> invokes;
Expand Down
5 changes: 2 additions & 3 deletions opt/object-sensitive-dce/ObjectSensitiveDcePass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ namespace uv = used_vars;
class CallGraphStrategy final : public call_graph::BuildStrategy {
public:
explicit CallGraphStrategy(const Scope& scope)
: m_scope(scope),
m_non_overridden_virtuals(hier::find_non_overridden_virtuals(scope)) {}
: m_scope(scope), m_non_overridden_virtuals(scope) {}

call_graph::CallSites get_callsites(const DexMethod* method) const override {
call_graph::CallSites callsites;
Expand Down Expand Up @@ -95,7 +94,7 @@ class CallGraphStrategy final : public call_graph::BuildStrategy {
}

const Scope& m_scope;
std::unordered_set<const DexMethod*> m_non_overridden_virtuals;
hier::NonOverriddenVirtuals m_non_overridden_virtuals;
};

static side_effects::InvokeToSummaryMap build_summary_map(
Expand Down
12 changes: 10 additions & 2 deletions test/unit/HierarchyUtilTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,16 @@ TEST_F(RedexTest, findNonOverriddenVirtuals) {

ext_cc.create();

const auto& non_overridden = find_non_overridden_virtuals({cls});
EXPECT_THAT(non_overridden,
NonOverriddenVirtuals non_overridden_virtuals({cls});
std::unordered_set<DexMethod*> set;
g_redex->walk_type_class([&](const DexType*, const DexClass* cls) {
for (auto* method : cls->get_vmethods()) {
if (non_overridden_virtuals.count(method)) {
set.insert(method);
}
}
});
EXPECT_THAT(set,
::testing::UnorderedElementsAre(final_method, nonfinal_method,
ext_final_method));
}

0 comments on commit d8f2552

Please sign in to comment.