diff options
author | 2018-06-18 13:33:55 -0700 | |
---|---|---|
committer | 2018-06-19 17:11:21 -0700 | |
commit | 89de444350fdbd7df4fb4d95bb71f00e6673b466 (patch) | |
tree | 57fa7214c4059afcfd11aa35a81999b8558b91b8 | |
parent | e383d23918db4eede30c3d78589d4639de3ec446 (diff) |
ART: Fix some performance-X tidy
Fix performance-for-range-copy, performance-unnecessary-copy-initialization
and performance-unnecessary-value-param issues.
Test: mmma art
Test: m test-art-host
Change-Id: I43d8736fc541030a3c61f66aeee0b9c2f1d295f7
-rw-r--r-- | dex2oat/dex2oat_test.cc | 6 | ||||
-rw-r--r-- | dex2oat/linker/arm/relative_patcher_arm_base.cc | 2 | ||||
-rw-r--r-- | dexlayout/dexlayout_test.cc | 4 | ||||
-rw-r--r-- | imgdiag/imgdiag.cc | 8 | ||||
-rw-r--r-- | libartbase/base/variant_map_test.cc | 2 | ||||
-rw-r--r-- | libdexfile/dex/dex_instruction_test.cc | 4 | ||||
-rw-r--r-- | libprofile/profile/profile_compilation_info.cc | 2 | ||||
-rw-r--r-- | profman/profman.cc | 2 | ||||
-rw-r--r-- | runtime/gc/collector/concurrent_copying.cc | 16 | ||||
-rw-r--r-- | runtime/gc/collector/concurrent_copying.h | 2 | ||||
-rw-r--r-- | runtime/subtype_check_info_test.cc | 4 | ||||
-rw-r--r-- | runtime/subtype_check_test.cc | 8 | ||||
-rw-r--r-- | sigchainlib/sigchain_test.cc | 2 | ||||
-rw-r--r-- | tools/wrapagentproperties/wrapagentproperties.cc | 2 |
14 files changed, 33 insertions, 31 deletions
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc index a060fd2e36..ad44624f76 100644 --- a/dex2oat/dex2oat_test.cc +++ b/dex2oat/dex2oat_test.cc @@ -1768,7 +1768,7 @@ TEST_F(Dex2oatTest, CompactDexGenerationFailureMultiDex) { writer.Finish(); ASSERT_EQ(apk_file.GetFile()->Flush(), 0); } - const std::string dex_location = apk_file.GetFilename(); + const std::string& dex_location = apk_file.GetFilename(); const std::string odex_location = GetOdexDir() + "/output.odex"; GenerateOdexForTest(dex_location, odex_location, @@ -1974,7 +1974,7 @@ TEST_F(Dex2oatTest, QuickenedInput) { << "Failed to find candidate code item with only one code unit in last instruction."; }); - std::string dex_location = temp_dex.GetFilename(); + const std::string& dex_location = temp_dex.GetFilename(); std::string odex_location = GetOdexDir() + "/quickened.odex"; std::string vdex_location = GetOdexDir() + "/quickened.vdex"; std::unique_ptr<File> vdex_output(OS::CreateEmptyFile(vdex_location.c_str())); @@ -2049,7 +2049,7 @@ TEST_F(Dex2oatTest, CompactDexInvalidSource) { writer.Finish(); ASSERT_EQ(invalid_dex.GetFile()->Flush(), 0); } - const std::string dex_location = invalid_dex.GetFilename(); + const std::string& dex_location = invalid_dex.GetFilename(); const std::string odex_location = GetOdexDir() + "/output.odex"; std::string error_msg; int status = GenerateOdexForTestWithStatus( diff --git a/dex2oat/linker/arm/relative_patcher_arm_base.cc b/dex2oat/linker/arm/relative_patcher_arm_base.cc index 7cb8ae55c5..a2ba339278 100644 --- a/dex2oat/linker/arm/relative_patcher_arm_base.cc +++ b/dex2oat/linker/arm/relative_patcher_arm_base.cc @@ -251,7 +251,7 @@ std::vector<debug::MethodDebugInfo> ArmBaseRelativePatcher::GenerateThunkDebugIn continue; } // Get the base name to use for the first occurrence of the thunk. - std::string base_name = data.GetDebugName(); + const std::string& base_name = data.GetDebugName(); for (size_t i = start, num = data.NumberOfThunks(); i != num; ++i) { debug::MethodDebugInfo info = {}; if (i == 0u) { diff --git a/dexlayout/dexlayout_test.cc b/dexlayout/dexlayout_test.cc index f148b94f3d..2b1352db16 100644 --- a/dexlayout/dexlayout_test.cc +++ b/dexlayout/dexlayout_test.cc @@ -468,7 +468,7 @@ class DexLayoutTest : public CommonRuntimeTest { } std::vector<std::string> test_files = { dex_file, profile_file, output_dex, second_output_dex }; - for (auto test_file : test_files) { + for (const std::string& test_file : test_files) { if (!UnlinkFile(test_file)) { return false; } @@ -501,7 +501,7 @@ class DexLayoutTest : public CommonRuntimeTest { } std::vector<std::string> dex_files = { input_dex, output_dex }; - for (auto dex_file : dex_files) { + for (const std::string& dex_file : dex_files) { if (!UnlinkFile(dex_file)) { return false; } diff --git a/imgdiag/imgdiag.cc b/imgdiag/imgdiag.cc index ddb8fe1302..fccc326a9d 100644 --- a/imgdiag/imgdiag.cc +++ b/imgdiag/imgdiag.cc @@ -335,7 +335,7 @@ class ImgObjectVisitor : public ObjectVisitor { using ComputeDirtyFunc = std::function<void(mirror::Object* object, const uint8_t* begin_image_ptr, const std::set<size_t>& dirty_pages)>; - ImgObjectVisitor(ComputeDirtyFunc dirty_func, + ImgObjectVisitor(const ComputeDirtyFunc& dirty_func, const uint8_t* begin_image_ptr, const std::set<size_t>& dirty_pages) : dirty_func_(dirty_func), @@ -356,7 +356,7 @@ class ImgObjectVisitor : public ObjectVisitor { } private: - ComputeDirtyFunc dirty_func_; + const ComputeDirtyFunc& dirty_func_; const uint8_t* begin_image_ptr_; const std::set<size_t>& dirty_pages_; }; @@ -646,7 +646,7 @@ class ImgArtMethodVisitor : public ArtMethodVisitor { using ComputeDirtyFunc = std::function<void(ArtMethod*, const uint8_t*, const std::set<size_t>&)>; - ImgArtMethodVisitor(ComputeDirtyFunc dirty_func, + ImgArtMethodVisitor(const ComputeDirtyFunc& dirty_func, const uint8_t* begin_image_ptr, const std::set<size_t>& dirty_pages) : dirty_func_(dirty_func), @@ -658,7 +658,7 @@ class ImgArtMethodVisitor : public ArtMethodVisitor { } private: - ComputeDirtyFunc dirty_func_; + const ComputeDirtyFunc& dirty_func_; const uint8_t* begin_image_ptr_; const std::set<size_t>& dirty_pages_; }; diff --git a/libartbase/base/variant_map_test.cc b/libartbase/base/variant_map_test.cc index 4677b6d3b3..f2da3389b1 100644 --- a/libartbase/base/variant_map_test.cc +++ b/libartbase/base/variant_map_test.cc @@ -108,7 +108,7 @@ TEST(VariantMaps, RuleOfFive) { EXPECT_EQ(size_t(2), fmFilled.Size()); // Test copy constructor - FruitMap fmEmptyCopy(fmEmpty); + FruitMap fmEmptyCopy(fmEmpty); // NOLINT EXPECT_EQ(size_t(0), fmEmptyCopy.Size()); // Test copy constructor diff --git a/libdexfile/dex/dex_instruction_test.cc b/libdexfile/dex/dex_instruction_test.cc index c944085b9e..6ce9dbafc8 100644 --- a/libdexfile/dex/dex_instruction_test.cc +++ b/libdexfile/dex/dex_instruction_test.cc @@ -135,7 +135,7 @@ TEST(Instruction, PropertiesOf4rcc) { static void Build35c(uint16_t* out, Instruction::Code code, uint16_t method_idx, - std::vector<uint16_t> args) { + const std::vector<uint16_t>& args) { out[0] = 0; out[0] |= (args.size() << 12); out[0] |= static_cast<uint16_t>(code); @@ -152,7 +152,7 @@ static void Build35c(uint16_t* out, static std::string DumpInst35c(Instruction::Code code, uint16_t method_idx, - std::vector<uint16_t> args) { + const std::vector<uint16_t>& args) { uint16_t inst[6] = {}; Build35c(inst, code, method_idx, args); return Instruction::At(inst)->DumpString(nullptr); diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc index 748e24e27c..1bb84b18dc 100644 --- a/libprofile/profile/profile_compilation_info.cc +++ b/libprofile/profile/profile_compilation_info.cc @@ -1383,7 +1383,7 @@ bool ProfileCompilationInfo::RemapProfileIndex( // the current profile info. // Note that the number of elements should be very small, so this should not // be a performance issue. - for (const ProfileLineHeader other_profile_line_header : profile_line_headers) { + for (const ProfileLineHeader& other_profile_line_header : profile_line_headers) { if (!filter_fn(other_profile_line_header.dex_location, other_profile_line_header.checksum)) { continue; } diff --git a/profman/profman.cc b/profman/profman.cc index 5fbce66412..9b470973c6 100644 --- a/profman/profman.cc +++ b/profman/profman.cc @@ -389,7 +389,7 @@ class ProfMan FINAL { } bool OpenApkFilesFromLocations( - std::function<void(std::unique_ptr<const DexFile>&&)> process_fn) { + const std::function<void(std::unique_ptr<const DexFile>&&)>& process_fn) { bool use_apk_fd_list = !apks_fd_.empty(); if (use_apk_fd_list) { // Get the APKs from the collection of FDs. diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc index 10fa8c508b..cbd98800f4 100644 --- a/runtime/gc/collector/concurrent_copying.cc +++ b/runtime/gc/collector/concurrent_copying.cc @@ -1822,7 +1822,7 @@ void ConcurrentCopying::ReclaimPhase() { std::string ConcurrentCopying::DumpReferenceInfo(mirror::Object* ref, const char* ref_name, - std::string indent) { + const char* indent) { std::ostringstream oss; oss << indent << heap_->GetVerification()->DumpObjectInfo(ref, ref_name) << '\n'; if (ref != nullptr) { @@ -1846,13 +1846,13 @@ std::string ConcurrentCopying::DumpHeapReference(mirror::Object* obj, MemberOffset offset, mirror::Object* ref) { std::ostringstream oss; - std::string indent = " "; - oss << indent << "Invalid reference: ref=" << ref + constexpr const char* kIndent = " "; + oss << kIndent << "Invalid reference: ref=" << ref << " referenced from: object=" << obj << " offset= " << offset << '\n'; // Information about `obj`. - oss << DumpReferenceInfo(obj, "obj", indent) << '\n'; + oss << DumpReferenceInfo(obj, "obj", kIndent) << '\n'; // Information about `ref`. - oss << DumpReferenceInfo(ref, "ref", indent); + oss << DumpReferenceInfo(ref, "ref", kIndent); return oss.str(); } @@ -1928,10 +1928,10 @@ class RootPrinter { std::string ConcurrentCopying::DumpGcRoot(mirror::Object* ref) { std::ostringstream oss; - std::string indent = " "; - oss << indent << "Invalid GC root: ref=" << ref << '\n'; + constexpr const char* kIndent = " "; + oss << kIndent << "Invalid GC root: ref=" << ref << '\n'; // Information about `ref`. - oss << DumpReferenceInfo(ref, "ref", indent); + oss << DumpReferenceInfo(ref, "ref", kIndent); return oss.str(); } diff --git a/runtime/gc/collector/concurrent_copying.h b/runtime/gc/collector/concurrent_copying.h index f1e7e2fd23..448525d013 100644 --- a/runtime/gc/collector/concurrent_copying.h +++ b/runtime/gc/collector/concurrent_copying.h @@ -242,7 +242,7 @@ class ConcurrentCopying : public GarbageCollector { REQUIRES_SHARED(Locks::mutator_lock_); // Dump information about reference `ref` and return it as a string. // Use `ref_name` to name the reference in messages. Each message is prefixed with `indent`. - std::string DumpReferenceInfo(mirror::Object* ref, const char* ref_name, std::string indent = "") + std::string DumpReferenceInfo(mirror::Object* ref, const char* ref_name, const char* indent = "") REQUIRES_SHARED(Locks::mutator_lock_); // Dump information about heap reference `ref`, referenced from object `obj` at offset `offset`, // and return it as a string. diff --git a/runtime/subtype_check_info_test.cc b/runtime/subtype_check_info_test.cc index 91fcc07d65..e40bca57fe 100644 --- a/runtime/subtype_check_info_test.cc +++ b/runtime/subtype_check_info_test.cc @@ -121,11 +121,11 @@ struct SubtypeCheckInfoTest : public ::testing::Test { return SubtypeCheckInfo::MakeUnchecked(bs, overflow, depth); } - static bool HasNext(SubtypeCheckInfo io) { + static bool HasNext(const SubtypeCheckInfo& io) { return io.HasNext(); } - static BitString GetPathToRoot(SubtypeCheckInfo io) { + static BitString GetPathToRoot(const SubtypeCheckInfo& io) { return io.GetPathToRoot(); } diff --git a/runtime/subtype_check_test.cc b/runtime/subtype_check_test.cc index 979fa42a57..666bf812f5 100644 --- a/runtime/subtype_check_test.cc +++ b/runtime/subtype_check_test.cc @@ -654,13 +654,15 @@ void EnsureStateChangedTestRecursive( MockClass* klass, size_t cur_depth, size_t total_depth, - std::vector<std::pair<SubtypeCheckInfo::State, SubtypeCheckInfo::State>> transitions) { + const std::vector<std::pair<SubtypeCheckInfo::State, SubtypeCheckInfo::State>>& transitions) { MockScopedLockSubtypeCheck lock_a; MockScopedLockMutator lock_b; using SCTree = MockSubtypeCheck; ASSERT_EQ(cur_depth, klass->Depth()); - ApplyTransition(SCTree::Lookup(klass), transitions[cur_depth].first, transitions[cur_depth].second); + ApplyTransition(SCTree::Lookup(klass), + transitions[cur_depth].first, + transitions[cur_depth].second); if (total_depth == cur_depth + 1) { return; @@ -676,7 +678,7 @@ void EnsureStateChangedTestRecursive( void EnsureStateChangedTest( MockClass* root, size_t depth, - std::vector<std::pair<SubtypeCheckInfo::State, SubtypeCheckInfo::State>> transitions) { + const std::vector<std::pair<SubtypeCheckInfo::State, SubtypeCheckInfo::State>>& transitions) { ASSERT_EQ(depth, transitions.size()); EnsureStateChangedTestRecursive(root, /*cur_depth*/0u, depth, transitions); diff --git a/sigchainlib/sigchain_test.cc b/sigchainlib/sigchain_test.cc index 1d1e54f127..9584ded65f 100644 --- a/sigchainlib/sigchain_test.cc +++ b/sigchainlib/sigchain_test.cc @@ -70,7 +70,7 @@ class SigchainTest : public ::testing::Test { }; -static void TestSignalBlocking(std::function<void()> fn) { +static void TestSignalBlocking(const std::function<void()>& fn) { // Unblock SIGSEGV, make sure it stays unblocked. sigset64_t mask; sigemptyset64(&mask); diff --git a/tools/wrapagentproperties/wrapagentproperties.cc b/tools/wrapagentproperties/wrapagentproperties.cc index 8b4b062cf5..77e19e691a 100644 --- a/tools/wrapagentproperties/wrapagentproperties.cc +++ b/tools/wrapagentproperties/wrapagentproperties.cc @@ -245,7 +245,7 @@ enum class StartType { static jint CallNextAgent(StartType start, ProxyJavaVM* vm, - std::string options, + const std::string& options, void* reserved) { // TODO It might be good to set it up so that the library is unloaded even if no jvmtiEnv's are // created but this isn't expected to be common so we will just not bother. |