From 3915a26c0f29fa0f669620c1c2b316f577bdad1b Mon Sep 17 00:00:00 2001 From: Koby Chan Date: Thu, 3 Oct 2024 12:23:03 -0700 Subject: [PATCH] Add violation for cold entry blocks in hot methods Summary: Adds violation logging for cold entry blocks in hot methods More formally, we expect that every source block that is the first source block in the entry block of a method must be hot if the method itself is hot. Otherwise it is a violation. Reviewed By: jimmycFB Differential Revision: D63795748 fbshipit-source-id: 28ece858f0189aab17e8c130bc933cdf3966ccea --- libredex/SourceBlocks.cpp | 76 +++++++++++++++++++++++++++++++++++++++ libredex/SourceBlocks.h | 1 + 2 files changed, 77 insertions(+) diff --git a/libredex/SourceBlocks.cpp b/libredex/SourceBlocks.cpp index 7af0ef0b9c..afc5dad937 100644 --- a/libredex/SourceBlocks.cpp +++ b/libredex/SourceBlocks.cpp @@ -19,6 +19,7 @@ #include "ControlFlow.h" #include "Debug.h" #include "DexClass.h" +#include "DexStructure.h" #include "Dominators.h" #include "IRList.h" #include "IROpcode.h" @@ -1013,6 +1014,8 @@ struct ViolationsHelper::ViolationsHelperImpl { return chain_and_dom_violations_cfg(cfg); case Violation::kUncoveredSourceBlocks: return uncovered_source_blocks_violations_cfg(cfg); + case Violation::kHotMethodColdEntry: + return hot_method_cold_entry_violations_cfg(cfg); } not_reached(); } @@ -1172,6 +1175,25 @@ struct ViolationsHelper::ViolationsHelperImpl { return sum; } + static size_t hot_method_cold_entry_violations_cfg( + cfg::ControlFlowGraph& cfg) { + size_t sum{0}; + auto* entry_block = cfg.entry_block(); + if (entry_block == nullptr) { + return 0; + } + auto* sb = get_first_source_block(entry_block); + if (sb == nullptr) { + return 0; + } + sb->foreach_val([&sum](const auto& val) { + if (val->appear100 != 0 && val->val == 0) { + sum++; + } + }); + return sum; + } + void print_all() const { for (const auto& m_str : print) { auto* m = DexMethod::get_method(m_str); @@ -1336,6 +1358,60 @@ struct ViolationsHelper::ViolationsHelperImpl { print_cfg_with_violations(m); return; } + case Violation::kHotMethodColdEntry: { + struct HotMethodColdEntry { + bool is_entry_block{false}; + bool first_in_block{false}; + + explicit HotMethodColdEntry(cfg::ControlFlowGraph&) {} + + void mie_before(std::ostream&, const MethodItemEntry&) {} + void mie_after(std::ostream& os, const MethodItemEntry& mie) { + if (mie.type != MFLOW_SOURCE_BLOCK || !is_entry_block || + !first_in_block) { + return; + } + first_in_block = false; + + auto* sb = mie.src_block.get(); + bool violation_found_in_head{false}; + sb->foreach_val([&violation_found_in_head](const auto& val) { + if (val->appear100 != 0 && val->val == 0) { + violation_found_in_head = true; + } + }); + if (violation_found_in_head) { + os << " !!! HEAD SB: METHOD IS HOT BUT ENTRY IS COLD"; + } + + bool violation_found_in_chain{false}; + for (auto* cur_sb = sb->next.get(); cur_sb != nullptr; + cur_sb = cur_sb->next.get()) { + cur_sb->foreach_val([&violation_found_in_chain](const auto& val) { + if (val->appear100 != 0 && val->val == 0) { + violation_found_in_chain = true; + } + }); + } + if (violation_found_in_chain) { + os << " !!! CHAIN SB: METHOD IS HOT BUT ENTRY IS COLD"; + } + os << "\n"; + } + + void start_block(std::ostream&, cfg::Block* b) { + if (b->preds().empty()) { + is_entry_block = true; + } else { + is_entry_block = false; + } + first_in_block = true; + } + void end_block(std::ostream&, cfg::Block*) {} + }; + print_cfg_with_violations(m); + return; + } } not_reached(); } diff --git a/libredex/SourceBlocks.h b/libredex/SourceBlocks.h index ce08439ef5..2e29489c62 100644 --- a/libredex/SourceBlocks.h +++ b/libredex/SourceBlocks.h @@ -490,6 +490,7 @@ struct ViolationsHelper { kHotImmediateDomNotHot, kChainAndDom, kUncoveredSourceBlocks, + kHotMethodColdEntry, }; ViolationsHelper(Violation v,