Expanded the performance lints (flutter#47868)

fixes flutter/flutter#137372 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
suzp1984 · Nov 9, 2023 · e80af85 · e80af85
1 parent d4a51fa
commit e80af85
Show file tree

Hide file tree

Showing 29 changed files with 37 additions and 25 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -22,7 +22,11 @@ Checks: >-
   -clang-analyzer-nullability.NullablePassedToNonnull,
   -clang-analyzer-nullability.NullReturnedFromNonnull,
   -clang-analyzer-nullability.NullableReturnedFromNonnull,
+  performance-for-range-copy,
+  performance-inefficient-vector-operation,
   performance-move-const-arg,
+  performance-move-constructor-init,
+  performance-unnecessary-copy-initialization,
   performance-unnecessary-value-param
 
 CheckOptions:

diff --git a/display_list/geometry/dl_rtree_unittests.cc b/display_list/geometry/dl_rtree_unittests.cc
@@ -307,7 +307,7 @@ TEST(DisplayListRTree, Region) {
     rect[i] = SkRect::MakeXYWH(i * 10, i * 10, 20, 20);
   }
   DlRTree rtree(rect, 9);
-  auto region = rtree.region();
+  const auto& region = rtree.region();
   auto rects = region.getRects(true);
   std::vector<SkIRect> expected_rects{
       SkIRect::MakeLTRB(0, 0, 20, 10),    SkIRect::MakeLTRB(0, 10, 30, 20),

diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc
@@ -2829,7 +2829,7 @@ class DisplayListRenderingTestBase : public BaseT,
       FML_LOG(INFO) << CanvasCompareTester::ImpellerFailureImages.size()
                     << " images saved in "
                     << CanvasCompareTester::ImpellerFailureImageDirectory;
-      for (auto filename : CanvasCompareTester::ImpellerFailureImages) {
+      for (const auto& filename : CanvasCompareTester::ImpellerFailureImages) {
         FML_LOG(INFO) << "  " << filename;
       }
       FML_LOG(INFO);

diff --git a/fml/memory/ref_counted_unittest.cc b/fml/memory/ref_counted_unittest.cc
@@ -128,6 +128,7 @@ TEST(RefCountedTest, Constructors) {
     was_destroyed = false;
     RefPtr<MyClass> r1(MakeRefCounted<MyClass>(&created, &was_destroyed));
     // Copy.
+    // NOLINTNEXTLINE(performance-unnecessary-copy-initialization)
     RefPtr<MyClass> r2(r1);
     EXPECT_TRUE(created);
     EXPECT_EQ(created, r1.get());
@@ -543,6 +544,7 @@ TEST(RefCountedTest, Mix) {
     RefPtr<MyClass> r3 = r2;
     EXPECT_FALSE(created->HasOneRef());
     {
+      // NOLINTNEXTLINE(performance-unnecessary-copy-initialization)
       RefPtr<MyClass> r4(r3);
       r2 = nullptr;
       ASSERT_FALSE(was_destroyed);

diff --git a/fml/memory/weak_ptr_unittest.cc b/fml/memory/weak_ptr_unittest.cc
@@ -28,7 +28,7 @@ TEST(WeakPtrTest, CopyConstruction) {
   int data = 0;
   WeakPtrFactory<int> factory(&data);
   WeakPtr<int> ptr = factory.GetWeakPtr();
-  WeakPtr<int> ptr2(ptr);
+  const WeakPtr<int>& ptr2(ptr);
   EXPECT_EQ(&data, ptr.get());
   EXPECT_EQ(&data, ptr2.get());
 }

diff --git a/fml/message_loop_task_queues_benchmark.cc b/fml/message_loop_task_queues_benchmark.cc
@@ -32,12 +32,12 @@ static void BM_RegisterAndGetTasks(benchmark::State& state) {  // NOLINT
     CountDownLatch tasks_registered(num_task_queues);
     CountDownLatch tasks_done(num_task_queues);
 
+    threads.reserve(num_task_queues);
     for (int i = 0; i < num_task_queues; i++) {
       threads.emplace_back([task_runner_id = i, &task_queue, past, &tasks_done,
                             &tasks_registered]() {
         for (int j = 0; j < num_tasks_per_queue; j++) {
-          task_queue->RegisterTask(
-              TaskQueueId(task_runner_id), [] {}, past);
+          task_queue->RegisterTask(TaskQueueId(task_runner_id), [] {}, past);
         }
         tasks_registered.CountDown();
         tasks_registered.Wait();

diff --git a/impeller/aiks/paint_pass_delegate.cc b/impeller/aiks/paint_pass_delegate.cc
@@ -103,7 +103,7 @@ bool OpacityPeepholePassDelegate::CanCollapseIntoParentPass(
   std::vector<Rect> all_coverages;
   auto had_subpass = entity_pass->IterateUntilSubpass(
       [&all_coverages, &all_can_accept](Entity& entity) {
-        auto contents = entity.GetContents();
+        const auto& contents = entity.GetContents();
         if (!entity.CanInheritOpacity()) {
           all_can_accept = false;
           return false;

diff --git a/impeller/compiler/uniform_sorter.cc b/impeller/compiler/uniform_sorter.cc
@@ -20,7 +20,7 @@ std::vector<spirv_cross::ID> SortUniforms(
         if (var.storage != spv::StorageClassUniformConstant) {
           return;
         }
-        const auto type = compiler->get_type(var.basetype);
+        const auto& type = compiler->get_type(var.basetype);
         if (!type_filter.has_value() ||
             (include && type_filter.value() == type.basetype) ||
             (!include && type_filter.value() != type.basetype)) {

diff --git a/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc
@@ -1357,7 +1357,7 @@ TEST_P(DisplayListTest, MaskBlursApplyCorrectlyToColorSources) {
           stops.data(), flutter::DlTileMode::kClamp)};
 
   int offset = 100;
-  for (auto color_source : color_sources) {
+  for (const auto& color_source : color_sources) {
     flutter::DlPaint paint;
     paint.setColorSource(color_source);
     paint.setMaskFilter(blur_filter);

diff --git a/impeller/entity/contents/filters/filter_contents.cc b/impeller/entity/contents/filters/filter_contents.cc
@@ -240,7 +240,7 @@ std::optional<Rect> FilterContents::GetSourceCoverage(
   }
 
   std::optional<Rect> inputs_coverage;
-  for (auto input : inputs_) {
+  for (const auto& input : inputs_) {
     auto input_coverage = input->GetSourceCoverage(
         effect_transform, filter_input_coverage.value());
     if (!input_coverage.has_value()) {

diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc
@@ -171,7 +171,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
   size_t minimum_sampler_index = 100000000;
   size_t buffer_index = 0;
   size_t buffer_offset = 0;
-  for (auto uniform : runtime_stage_->GetUniforms()) {
+  for (const auto& uniform : runtime_stage_->GetUniforms()) {
     // TODO(113715): Populate this metadata once GLES is able to handle
     //               non-struct uniform names.
     std::shared_ptr<ShaderMetadata> metadata =
@@ -225,7 +225,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
   }
 
   size_t sampler_index = 0;
-  for (auto uniform : runtime_stage_->GetUniforms()) {
+  for (const auto& uniform : runtime_stage_->GetUniforms()) {
     // TODO(113715): Populate this metadata once GLES is able to handle
     //               non-struct uniform names.
     ShaderMetadata metadata;

diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc
@@ -28,6 +28,7 @@ std::shared_ptr<VerticesGeometry> CreateColorVertices(
     const std::vector<Color>& colors) {
   auto bounds = Rect::MakePointBounds(vertices.begin(), vertices.end());
   std::vector<uint16_t> indices = {};
+  indices.reserve(vertices.size());
   for (auto i = 0u; i < vertices.size(); i++) {
     indices.emplace_back(i);
   }

diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc
@@ -323,7 +323,7 @@ bool EntityPass::Render(ContentContext& renderer,
 
   IterateAllEntities([lazy_glyph_atlas =
                           renderer.GetLazyGlyphAtlas()](const Entity& entity) {
-    if (auto contents = entity.GetContents()) {
+    if (const auto& contents = entity.GetContents()) {
       contents->PopulateGlyphAtlas(lazy_glyph_atlas, entity.DeriveTextScale());
     }
     return true;

diff --git a/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/impeller/renderer/backend/gles/buffer_bindings_gles.cc
@@ -204,7 +204,7 @@ const std::vector<GLint>& BufferBindingsGLES::ComputeUniformLocations(
   // For each metadata member, look up the binding location and record
   // it in the binding map.
   auto& locations = binding_map_[metadata->name] = {};
-  for (const auto member : metadata->members) {
+  for (const auto& member : metadata->members) {
     if (member.type == ShaderType::kVoid) {
       // Void types are used for padding. We are obviously not going to find
       // mappings for these. Keep going.

diff --git a/impeller/renderer/pool_unittests.cc b/impeller/renderer/pool_unittests.cc
@@ -48,10 +48,11 @@ TEST(PoolTest, Overload) {
   Pool<Foobar> pool(1'000);
   {
     std::vector<std::shared_ptr<Foobar>> values;
+    values.reserve(20);
     for (int i = 0; i < 20; i++) {
       values.push_back(pool.Grab());
     }
-    for (auto value : values) {
+    for (const auto& value : values) {
       value->SetSize(100);
       pool.Recycle(value);
     }

diff --git a/lib/ui/painting/fragment_program.cc b/lib/ui/painting/fragment_program.cc
@@ -59,7 +59,7 @@ std::string FragmentProgram::initFromAsset(const std::string& asset_name) {
     runtime_effect_ = DlRuntimeEffect::MakeImpeller(
         std::make_unique<impeller::RuntimeStage>(std::move(runtime_stage)));
   } else {
-    auto code_mapping = runtime_stage.GetSkSLMapping();
+    const auto& code_mapping = runtime_stage.GetSkSLMapping();
     auto code_size = code_mapping->GetSize();
     const char* sksl =
         reinterpret_cast<const char*>(code_mapping->GetMapping());

diff --git a/lib/ui/painting/path_measure.cc b/lib/ui/painting/path_measure.cc
@@ -27,7 +27,7 @@ void CanvasPathMeasure::Create(Dart_Handle wrapper,
   fml::RefPtr<CanvasPathMeasure> pathMeasure =
       fml::MakeRefCounted<CanvasPathMeasure>();
   if (path) {
-    const SkPath skPath = path->path();
+    const SkPath& skPath = path->path();
     SkScalar resScale = 1;
     pathMeasure->path_measure_ =
         std::make_unique<SkContourMeasureIter>(skPath, forceClosed, resScale);

diff --git a/lib/ui/ui_benchmarks.cc b/lib/ui/ui_benchmarks.cc
@@ -90,7 +90,7 @@ static void BM_PathVolatilityTracker(benchmark::State& state) {
 
     fml::AutoResetWaitableEvent latch;
     task_runners.GetUITaskRunner()->PostTask([&]() {
-      for (auto path : paths) {
+      for (const auto& path : paths) {
         tracker.Track(path);
       }
       latch.Signal();

diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc
@@ -627,6 +627,7 @@ bool DartIsolate::LoadKernel(const std::shared_ptr<const fml::Mapping>& mapping,
 [[nodiscard]] bool DartIsolate::PrepareForRunningFromKernels(
     std::vector<std::unique_ptr<const fml::Mapping>> kernels) {
   std::vector<std::shared_ptr<const fml::Mapping>> shared_kernels;
+  shared_kernels.reserve(kernels.size());
   for (auto& kernel : kernels) {
     shared_kernels.emplace_back(std::move(kernel));
   }

diff --git a/runtime/service_protocol.cc b/runtime/service_protocol.cc
@@ -268,6 +268,7 @@ bool ServiceProtocol::HandleListViewsMethod(
     rapidjson::Document* response) const {
   fml::SharedLock lock(*handlers_mutex_);
   std::vector<std::pair<intptr_t, Handler::Description>> descriptions;
+  descriptions.reserve(handlers_.size());
   for (const auto& handler : handlers_) {
     descriptions.emplace_back(reinterpret_cast<intptr_t>(handler.first),
                               handler.second.Load());

diff --git a/shell/common/resource_cache_limit_calculator.cc b/shell/common/resource_cache_limit_calculator.cc
@@ -12,7 +12,7 @@ size_t ResourceCacheLimitCalculator::GetResourceCacheMaxBytes() {
                                    ? max_bytes_threshold_
                                    : std::numeric_limits<size_t>::max();
   std::vector<fml::WeakPtr<ResourceCacheLimitItem>> live_items;
-  for (auto item : items_) {
+  for (const auto& item : items_) {
     if (item) {
       live_items.push_back(item);
       max_bytes += item->GetResourceCacheLimit();

diff --git a/shell/common/serialization_callbacks.cc b/shell/common/serialization_callbacks.cc
@@ -34,7 +34,7 @@ struct ImageMetaData {
 } __attribute__((packed));
 
 sk_sp<SkData> SerializeImageWithoutData(SkImage* image, void* ctx) {
-  auto info = image->imageInfo();
+  const auto& info = image->imageInfo();
   SkDynamicMemoryWStream stream;
 
   ImageMetaData metadata = {info.width(), info.height(),

diff --git a/shell/common/shell.cc b/shell/common/shell.cc
@@ -2219,6 +2219,7 @@ void Shell::OnDisplayUpdates(std::vector<std::unique_ptr<Display>> displays) {
   FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());
 
   std::vector<DisplayData> display_data;
+  display_data.reserve(displays.size());
   for (const auto& display : displays) {
     display_data.push_back(display->GetDisplayData());
   }

diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc
@@ -3162,7 +3162,7 @@ TEST_F(ShellTest, AssetManagerMulti) {
       "bad1",
   };
 
-  for (auto filename : filenames) {
+  for (const auto& filename : filenames) {
     bool success = fml::WriteAtomically(asset_dir_fd, filename.c_str(),
                                         fml::DataMapping(filename));
     ASSERT_TRUE(success);

diff --git a/shell/common/switches.cc b/shell/common/switches.cc
@@ -491,7 +491,7 @@ Settings SettingsFromCommandLine(const fml::CommandLine& command_line) {
                                   &all_dart_flags)) {
     // Assume that individual flags are comma separated.
     std::vector<std::string> flags = ParseCommaDelimited(all_dart_flags);
-    for (auto flag : flags) {
+    for (const auto& flag : flags) {
       if (!IsAllowedDartVMFlag(flag)) {
         FML_LOG(FATAL) << "Encountered disallowed Dart VM flag: " << flag;
       }

diff --git a/shell/platform/common/accessibility_bridge.cc b/shell/platform/common/accessibility_bridge.cc
@@ -91,7 +91,7 @@ void AccessibilityBridge::CommitUpdates() {
   }
 
   for (size_t i = results.size(); i > 0; i--) {
-    for (SemanticsNode node : results[i - 1]) {
+    for (const SemanticsNode& node : results[i - 1]) {
       ConvertFlutterUpdate(node, update);
     }
   }
@@ -203,7 +203,7 @@ std::optional<ui::AXTreeUpdate>
 AccessibilityBridge::CreateRemoveReparentedNodesUpdate() {
   std::unordered_map<int32_t, ui::AXNodeData> updates;
 
-  for (auto node_update : pending_semantics_node_updates_) {
+  for (const auto& node_update : pending_semantics_node_updates_) {
     for (int32_t child_id : node_update.second.children_in_traversal_order) {
       // Skip nodes that don't exist or have a parent in the current tree.
       ui::AXNode* child = tree_->GetFromId(child_id);

diff --git a/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm b/shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMac.mm
@@ -33,7 +33,7 @@
   ui::AXNode* ax_node = targeted_event.node;
   std::vector<AccessibilityBridgeMac::NSAccessibilityEvent> events =
       MacOSEventsFromAXEvent(targeted_event.event_params.event, *ax_node);
-  for (AccessibilityBridgeMac::NSAccessibilityEvent event : events) {
+  for (const AccessibilityBridgeMac::NSAccessibilityEvent& event : events) {
     if (event.user_info != nil) {
       DispatchMacOSNotificationWithUserInfo(event.target, event.name, event.user_info);
     } else {

diff --git a/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterMutatorViewTest.mm
@@ -34,6 +34,7 @@ void ApplyFlutterLayer(FlutterMutatorView* view,
   flutterPlatformView.identifier = 0;
 
   std::vector<const FlutterPlatformViewMutation*> mutationPointers;
+  mutationPointers.reserve(mutations.size());
   for (auto& mutation : mutations) {
     mutationPointers.push_back(&mutation);
   }

diff --git a/shell/platform/embedder/tests/embedder_test_context.cc b/shell/platform/embedder/tests/embedder_test_context.cc
@@ -111,7 +111,7 @@ VoidCallback EmbedderTestContext::GetIsolateCreateCallbackHook() {
 }
 
 void EmbedderTestContext::FireIsolateCreateCallbacks() {
-  for (auto closure : isolate_create_callbacks_) {
+  for (const auto& closure : isolate_create_callbacks_) {
     closure();
   }
 }