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
LineageOS · Feb 9, 2024 · 554d484 · 554d484
1 parent 158c3ff
commit 554d484
Show file tree

Hide file tree

Showing 12 changed files with 35 additions and 30 deletions.
diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc
@@ -248,7 +248,7 @@ class ValueSet : public ArenaObject<kArenaAllocGvn> {
   // Iterates over buckets with impure instructions (even indices) and deletes
   // the ones on which 'cond' returns true.
   template<typename Functor>
-  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
@@ -35,7 +35,7 @@ static int32_t FillIntrinsicsObjects(
     ObjPtr<mirror::ObjectArray<mirror::Object>> 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<mirror::ObjectArray<mirror::Object>> cache =

diff --git a/compiler/optimizing/intrinsics_arm64.cc b/compiler/optimizing/intrinsics_arm64.cc
@@ -3969,7 +3969,7 @@ template<typename OP>
 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
@@ -2727,7 +2727,7 @@ struct ScopedRestoreHeapValues {
   }
 
   template<typename Func>
-  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
@@ -110,7 +110,7 @@ class StackMapStream : public DeletableArenaObject<kArenaAllocStackMapStream> {
 
   // Invokes the callback with pointer of each BitTableBuilder field.
   template<typename Callback>
-  void ForEachBitTable(Callback callback) {
+  void ForEachBitTable(Callback&& callback) {
     size_t index = 0;
     callback(index++, &stack_maps_);
     callback(index++, &register_masks_);

diff --git a/libartbase/base/safe_map.h b/libartbase/base/safe_map.h
@@ -149,7 +149,7 @@ class SafeMap {
   }
 
   template <typename CreateFn>
-  V& GetOrCreate(const K& k, CreateFn create) {
+  V& GetOrCreate(const K& k, CreateFn&& create) {
     static_assert(std::is_same_v<V, std::invoke_result_t<CreateFn>>,
                   "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
@@ -202,20 +202,22 @@ template<typename NewLocalVisitor>
 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<NewLocalVisitor>(new_local));
 }
 
 template <typename Visitor>
-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>(visitor)) :
+             0u;
 }
 
 inline bool CodeItemDebugInfoAccessor::GetLineNumForPc(const uint32_t address,
@@ -233,13 +235,13 @@ inline bool CodeItemDebugInfoAccessor::GetLineNumForPc(const uint32_t address,
 }
 
 template <typename Visitor>
-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>(visitor));
 }
 
 }  // namespace art

diff --git a/libdexfile/dex/code_item_accessors.h b/libdexfile/dex/code_item_accessors.h
@@ -164,15 +164,15 @@ class CodeItemDebugInfoAccessor : public CodeItemDataAccessor {
   template<typename NewLocalVisitor>
   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 <typename Visitor>
-  uint32_t VisitParameterNames(const Visitor& visitor) const;
+  uint32_t VisitParameterNames(Visitor&& visitor) const;
 
   template <typename Visitor>
-  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
@@ -427,8 +427,8 @@ bool DexFile::DecodeDebugLocalInfo(uint32_t registers_size,
 
 template<typename DexDebugNewPosition, typename IndexToStringData>
 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<ClassIterator> DexFile::GetClasses() const {
 // Returns the line number
 template <typename Visitor>
 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
@@ -787,8 +787,8 @@ class DexFile {
   // Returns false if there is no debugging information or if it cannot be decoded.
   template<typename DexDebugNewPosition, typename IndexToStringData>
   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 <typename Visitor>
   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
@@ -119,7 +119,7 @@ class ElfDebugReader {
   }
 
   template <typename VisitSym>
-  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<VisitSym>(visit_sym));
     }
   }
 
   template <typename VisitSym>
-  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 <typename VisitCIE, typename VisitFDE>
-  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<VisitCIE>(visit_cie),
+                                             std::forward<VisitFDE>(visit_fde));
     }
   }
 

diff --git 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.