From 554d48465f31b0e073f2c53a09ed9851f8b3e692 Mon Sep 17 00:00:00 2001 From: Santiago Aboy Solanes Date: Wed, 7 Feb 2024 17:15:16 +0000 Subject: Pass functors as rvalues when possible On local compiles I saw that DeleteAllImpureWhich was the third most time consuming method, using pprofs sorting in bottom-up. By passing the functor it uses as rvalue, the method speeds up ~10% and it drops from the third most time consuming to the fourth. The other modified methods in the CL are not showing up in the pprof profile that I took, but changing them to use rvalues shouldn't affect them negatively. Test: Locally compile, take a trace, and observe time spent Test: art/test/testrunner/testrunner.py --host --64 --optimizing -b Change-Id: I6c363d5601fd4865f4e7881e64b883bd6bbedb69 --- compiler/optimizing/gvn.cc | 2 +- compiler/optimizing/intrinsic_objects.cc | 2 +- compiler/optimizing/intrinsics_arm64.cc | 2 +- compiler/optimizing/load_store_elimination.cc | 2 +- compiler/optimizing/stack_map_stream.h | 2 +- libartbase/base/safe_map.h | 2 +- libdexfile/dex/code_item_accessors-inl.h | 14 ++++++++------ libdexfile/dex/code_item_accessors.h | 6 +++--- libdexfile/dex/dex_file-inl.h | 6 +++--- libdexfile/dex/dex_file.h | 6 +++--- libelffile/elf/elf_debug_reader.h | 11 ++++++----- openjdkjvmti/ti_method.cc | 10 ++++++---- 12 files changed, 35 insertions(+), 30 deletions(-) diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc index a6ca057cfc..9113860387 100644 --- a/compiler/optimizing/gvn.cc +++ b/compiler/optimizing/gvn.cc @@ -248,7 +248,7 @@ class ValueSet : public ArenaObject { // Iterates over buckets with impure instructions (even indices) and deletes // the ones on which 'cond' returns true. template - void DeleteAllImpureWhich(Functor cond) { + void DeleteAllImpureWhich(Functor&& cond) { for (size_t i = 0; i < num_buckets_; i += 2) { Node* node = buckets_[i]; Node* previous = nullptr; diff --git a/compiler/optimizing/intrinsic_objects.cc b/compiler/optimizing/intrinsic_objects.cc index 6c799d4132..c625d435ae 100644 --- a/compiler/optimizing/intrinsic_objects.cc +++ b/compiler/optimizing/intrinsic_objects.cc @@ -35,7 +35,7 @@ static int32_t FillIntrinsicsObjects( ObjPtr> live_objects, int32_t expected_low, int32_t expected_high, - T type_check, + T&& type_check, int32_t index) REQUIRES_SHARED(Locks::mutator_lock_) { ObjPtr> cache = diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc index 2ae44cd4b0..56a5186d36 100644 --- a/compiler/optimizing/intrinsics_arm64.cc +++ b/compiler/optimizing/intrinsics_arm64.cc @@ -3969,7 +3969,7 @@ template void GenerateFP16Round(HInvoke* invoke, CodeGeneratorARM64* const codegen_, MacroAssembler* masm, - const OP roundOp) { + OP&& roundOp) { DCHECK(codegen_->GetInstructionSetFeatures().HasFP16()); LocationSummary* locations = invoke->GetLocations(); UseScratchRegisterScope scratch_scope(masm); diff --git a/compiler/optimizing/load_store_elimination.cc b/compiler/optimizing/load_store_elimination.cc index 38c1cfc0d2..09e23a8552 100644 --- a/compiler/optimizing/load_store_elimination.cc +++ b/compiler/optimizing/load_store_elimination.cc @@ -2727,7 +2727,7 @@ struct ScopedRestoreHeapValues { } template - void ForEachRecord(Func func) { + void ForEachRecord(Func&& func) { for (size_t blk_id : Range(to_restore_.size())) { for (size_t heap_loc : Range(to_restore_[blk_id].size())) { LSEVisitor::ValueRecord* vr = &to_restore_[blk_id][heap_loc]; diff --git a/compiler/optimizing/stack_map_stream.h b/compiler/optimizing/stack_map_stream.h index 8c7b1c01a7..a3daa29b4e 100644 --- a/compiler/optimizing/stack_map_stream.h +++ b/compiler/optimizing/stack_map_stream.h @@ -110,7 +110,7 @@ class StackMapStream : public DeletableArenaObject { // Invokes the callback with pointer of each BitTableBuilder field. template - void ForEachBitTable(Callback callback) { + void ForEachBitTable(Callback&& callback) { size_t index = 0; callback(index++, &stack_maps_); callback(index++, ®ister_masks_); diff --git a/libartbase/base/safe_map.h b/libartbase/base/safe_map.h index fa13fe0f68..c489bfd866 100644 --- a/libartbase/base/safe_map.h +++ b/libartbase/base/safe_map.h @@ -149,7 +149,7 @@ class SafeMap { } template - V& GetOrCreate(const K& k, CreateFn create) { + V& GetOrCreate(const K& k, CreateFn&& create) { static_assert(std::is_same_v>, "Argument `create` should return a value of type V."); auto lb = lower_bound(k); diff --git a/libdexfile/dex/code_item_accessors-inl.h b/libdexfile/dex/code_item_accessors-inl.h index b74046071f..4586a0ff7a 100644 --- a/libdexfile/dex/code_item_accessors-inl.h +++ b/libdexfile/dex/code_item_accessors-inl.h @@ -202,20 +202,22 @@ template inline bool CodeItemDebugInfoAccessor::DecodeDebugLocalInfo( bool is_static, uint32_t method_idx, - const NewLocalVisitor& new_local) const { + NewLocalVisitor&& new_local) const { return dex_file_->DecodeDebugLocalInfo(RegistersSize(), InsSize(), InsnsSizeInCodeUnits(), DebugInfoOffset(), is_static, method_idx, - new_local); + std::forward(new_local)); } template -inline uint32_t CodeItemDebugInfoAccessor::VisitParameterNames(const Visitor& visitor) const { +inline uint32_t CodeItemDebugInfoAccessor::VisitParameterNames(Visitor&& visitor) const { const uint8_t* stream = dex_file_->GetDebugInfoStream(DebugInfoOffset()); - return (stream != nullptr) ? DexFile::DecodeDebugInfoParameterNames(&stream, visitor) : 0u; + return (stream != nullptr) ? + DexFile::DecodeDebugInfoParameterNames(&stream, std::forward(visitor)) : + 0u; } inline bool CodeItemDebugInfoAccessor::GetLineNumForPc(const uint32_t address, @@ -233,13 +235,13 @@ inline bool CodeItemDebugInfoAccessor::GetLineNumForPc(const uint32_t address, } template -inline bool CodeItemDebugInfoAccessor::DecodeDebugPositionInfo(const Visitor& visitor) const { +inline bool CodeItemDebugInfoAccessor::DecodeDebugPositionInfo(Visitor&& visitor) const { return dex_file_->DecodeDebugPositionInfo( dex_file_->GetDebugInfoStream(DebugInfoOffset()), [this](uint32_t idx) { return dex_file_->StringDataByIdx(dex::StringIndex(idx)); }, - visitor); + std::forward(visitor)); } } // namespace art diff --git a/libdexfile/dex/code_item_accessors.h b/libdexfile/dex/code_item_accessors.h index 24296c8f1d..5952b2d7ea 100644 --- a/libdexfile/dex/code_item_accessors.h +++ b/libdexfile/dex/code_item_accessors.h @@ -164,15 +164,15 @@ class CodeItemDebugInfoAccessor : public CodeItemDataAccessor { template bool DecodeDebugLocalInfo(bool is_static, uint32_t method_idx, - const NewLocalVisitor& new_local) const; + NewLocalVisitor&& new_local) const; // Visit each parameter in the debug information. Returns the line number. // The argument of the Visitor is dex::StringIndex. template - uint32_t VisitParameterNames(const Visitor& visitor) const; + uint32_t VisitParameterNames(Visitor&& visitor) const; template - bool DecodeDebugPositionInfo(const Visitor& visitor) const; + bool DecodeDebugPositionInfo(Visitor&& visitor) const; bool GetLineNumForPc(const uint32_t pc, uint32_t* line_num) const; diff --git a/libdexfile/dex/dex_file-inl.h b/libdexfile/dex/dex_file-inl.h index 9291f6ee9e..b01b004e5b 100644 --- a/libdexfile/dex/dex_file-inl.h +++ b/libdexfile/dex/dex_file-inl.h @@ -427,8 +427,8 @@ bool DexFile::DecodeDebugLocalInfo(uint32_t registers_size, template bool DexFile::DecodeDebugPositionInfo(const uint8_t* stream, - const IndexToStringData& index_to_string_data, - const DexDebugNewPosition& position_functor) { + IndexToStringData&& index_to_string_data, + DexDebugNewPosition&& position_functor) { if (stream == nullptr) { return false; } @@ -514,7 +514,7 @@ inline IterationRange DexFile::GetClasses() const { // Returns the line number template inline uint32_t DexFile::DecodeDebugInfoParameterNames(const uint8_t** debug_info, - const Visitor& visitor) { + Visitor&& visitor) { uint32_t line = DecodeUnsignedLeb128(debug_info); const uint32_t parameters_size = DecodeUnsignedLeb128(debug_info); for (uint32_t i = 0; i < parameters_size; ++i) { diff --git a/libdexfile/dex/dex_file.h b/libdexfile/dex/dex_file.h index a7d288120d..8bcf1d803a 100644 --- a/libdexfile/dex/dex_file.h +++ b/libdexfile/dex/dex_file.h @@ -787,8 +787,8 @@ class DexFile { // Returns false if there is no debugging information or if it cannot be decoded. template static bool DecodeDebugPositionInfo(const uint8_t* stream, - const IndexToStringData& index_to_string_data, - const DexDebugNewPosition& position_functor); + IndexToStringData&& index_to_string_data, + DexDebugNewPosition&& position_functor); const char* GetSourceFile(const dex::ClassDef& class_def) const { if (!class_def.source_file_idx_.IsValid()) { @@ -893,7 +893,7 @@ class DexFile { template static uint32_t DecodeDebugInfoParameterNames(const uint8_t** debug_info, - const Visitor& visitor); + Visitor&& visitor); static inline bool StringEquals(const DexFile* df1, dex::StringIndex sidx1, const DexFile* df2, dex::StringIndex sidx2); diff --git a/libelffile/elf/elf_debug_reader.h b/libelffile/elf/elf_debug_reader.h index fc7ad5654b..05aa339c13 100644 --- a/libelffile/elf/elf_debug_reader.h +++ b/libelffile/elf/elf_debug_reader.h @@ -119,7 +119,7 @@ class ElfDebugReader { } template - void VisitFunctionSymbols(VisitSym visit_sym) { + void VisitFunctionSymbols(VisitSym&& visit_sym) { const Elf_Shdr* symtab = GetSection(".symtab"); const Elf_Shdr* strtab = GetSection(".strtab"); const Elf_Shdr* text = GetSection(".text"); @@ -135,12 +135,12 @@ class ElfDebugReader { } } if (gnu_debugdata_reader_ != nullptr) { - gnu_debugdata_reader_->VisitFunctionSymbols(visit_sym); + gnu_debugdata_reader_->VisitFunctionSymbols(std::forward(visit_sym)); } } template - void VisitDynamicSymbols(VisitSym visit_sym) { + void VisitDynamicSymbols(VisitSym&& visit_sym) { const Elf_Shdr* dynsym = GetSection(".dynsym"); const Elf_Shdr* dynstr = GetSection(".dynstr"); if (dynsym != nullptr && dynstr != nullptr) { @@ -153,7 +153,7 @@ class ElfDebugReader { } template - void VisitDebugFrame(VisitCIE visit_cie, VisitFDE visit_fde) { + void VisitDebugFrame(VisitCIE&& visit_cie, VisitFDE&& visit_fde) { const Elf_Shdr* debug_frame = GetSection(".debug_frame"); if (debug_frame != nullptr) { for (size_t offset = 0; offset < debug_frame->sh_size;) { @@ -169,7 +169,8 @@ class ElfDebugReader { } } if (gnu_debugdata_reader_ != nullptr) { - gnu_debugdata_reader_->VisitDebugFrame(visit_cie, visit_fde); + gnu_debugdata_reader_->VisitDebugFrame(std::forward(visit_cie), + std::forward(visit_fde)); } } diff --git a/openjdkjvmti/ti_method.cc b/openjdkjvmti/ti_method.cc index a460b39556..a1f25c466b 100644 --- a/openjdkjvmti/ti_method.cc +++ b/openjdkjvmti/ti_method.cc @@ -249,6 +249,7 @@ jvmtiError MethodUtil::GetLocalVariableTable(jvmtiEnv* env, return OK; }; + // To avoid defining visitor in the same line as the `if`. We define the lambda and use std::move. auto visitor = [&](const art::DexFile::LocalInfo& entry) { if (err != OK) { return; @@ -275,9 +276,8 @@ jvmtiError MethodUtil::GetLocalVariableTable(jvmtiEnv* env, }); }; - if (!accessor.DecodeDebugLocalInfo(art_method->IsStatic(), - art_method->GetDexMethodIndex(), - visitor)) { + if (!accessor.DecodeDebugLocalInfo( + art_method->IsStatic(), art_method->GetDexMethodIndex(), std::move(visitor))) { // Something went wrong with decoding the debug information. It might as well not be there. return ERR(ABSENT_INFORMATION); } @@ -754,6 +754,7 @@ jvmtiError CommonLocalVariableClosure::GetSlotType(art::ArtMethod* method, bool found = false; *type = art::Primitive::kPrimVoid; descriptor->clear(); + // To avoid defining visitor in the same line as the `if`. We define the lambda and use std::move. auto visitor = [&](const art::DexFile::LocalInfo& entry) { if (!found && entry.start_address_ <= dex_pc && entry.end_address_ > dex_pc && entry.reg_ == slot_) { @@ -762,7 +763,8 @@ jvmtiError CommonLocalVariableClosure::GetSlotType(art::ArtMethod* method, *descriptor = entry.descriptor_; } }; - if (!accessor.DecodeDebugLocalInfo(method->IsStatic(), method->GetDexMethodIndex(), visitor) || + if (!accessor.DecodeDebugLocalInfo( + method->IsStatic(), method->GetDexMethodIndex(), std::move(visitor)) || !found) { // Something went wrong with decoding the debug information. It might as well not be there. // Try to find the type with the verifier. -- cgit v1.2.3-59-g8ed1b