Make symbol loading more than 50x faster on Linux (#4840)

Remove N^2 redundant sorting of functions data views when loading symbols.
On Linux, this makes Unreal symbols load in ~5 seconds instead of more
than 4 minutes. On Windows the gains are a bit less because we are
limited by the poor performance of the DiaSdk for symbol loading.
An upcoming PR will introduce raw_pdb for another massive improvement.

src/ClientData/ModuleData.cpp

@@ -13,6 +13,7 @@
 #include <utility>
 
 #include "GrpcProtos/module.pb.h"
+#include "Introspection/Introspection.h"
 #include "OrbitBase/Logging.h"
 
 using orbit_grpc_protos::ModuleInfo;
@@ -232,6 +233,7 @@ bool ModuleData::AreAtLeastFallbackSymbolsLoaded() const {
 }
 
 void ModuleData::AddSymbols(const orbit_grpc_protos::ModuleSymbols& module_symbols) {
+  ORBIT_SCOPE_FUNCTION;
   absl::MutexLock lock(&mutex_);
   AddSymbolsInternal(module_symbols, SymbolCompleteness::kDebugSymbols);
 }
@@ -243,6 +245,8 @@ void ModuleData::AddFallbackSymbols(const orbit_grpc_protos::ModuleSymbols& modu
 
 void ModuleData::AddSymbolsInternal(const orbit_grpc_protos::ModuleSymbols& module_symbols,
                                     ModuleData::SymbolCompleteness completeness) {
+  ORBIT_SCOPE(
+      absl::StrFormat("AddSymbolsInternal [%u]", module_symbols.symbol_infos().size()).c_str());
   mutex_.AssertHeld();
   ORBIT_CHECK(loaded_symbols_completeness_ < completeness);
   functions_.clear();

src/DataViews/DataView.cpp

@@ -61,7 +61,9 @@ void DataView::OnSort(int column, std::optional<SortingOrder> new_order) {
   }
 
   {
-    ORBIT_SCOPE(absl::StrFormat("DoSort column[%i]", sorting_column_).c_str());
+    ORBIT_SCOPE(
+        absl::StrFormat("DoSort column[%i] %s", sorting_column_, orbit_data_views::ToString(type_))
+            .c_str());
     DoSort();
   }
 }

src/DataViews/FunctionsDataView.cpp

@@ -126,6 +126,7 @@ std::string FunctionsDataView::GetValue(int row, int column) {
   }
 
 void FunctionsDataView::DoSort() {
+  ORBIT_SCOPE_FUNCTION;
   // TODO(antonrohr): This sorting function can take a lot of time when a large
   // number of functions is used (several seconds). This function is currently
   // executed on the main thread and therefore freezes the UI and interrupts the
@@ -245,20 +246,21 @@ void FunctionsDataView::DoFilter() {
 
 void FunctionsDataView::AddFunctions(
     std::vector<const orbit_client_data::FunctionInfo*> functions) {
+  ORBIT_SCOPE_FUNCTION;
   functions_.insert(functions_.end(), functions.begin(), functions.end());
-  OnDataChanged();
 }
 
 void FunctionsDataView::RemoveFunctionsOfModule(std::string_view module_path) {
+  ORBIT_SCOPE_FUNCTION;
   functions_.erase(std::remove_if(functions_.begin(), functions_.end(),
                                   [&module_path](const FunctionInfo* function_info) {
                                     return function_info->module_path() == module_path;
                                   }),
                    functions_.end());
-  OnDataChanged();
 }
 
 void FunctionsDataView::ClearFunctions() {
+  ORBIT_SCOPE_FUNCTION;
   functions_.clear();
   OnDataChanged();
 }

src/DataViews/FunctionsDataViewTest.cpp

@@ -116,6 +116,8 @@ struct FunctionsDataViewTest : public testing::Test {
     module_info2.set_load_bias(0x6000);
     module_info2.set_address_start(0x3456);
     module_infos_.emplace_back(std::move(module_info2));
+
+    view_.OnDataChanged();
   }
 
  protected:
@@ -161,12 +163,14 @@ TEST_F(FunctionsDataViewTest, FunctionNameIsDisplayName) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
   EXPECT_EQ(view_.GetValue(0, 1), functions_[0].pretty_name());
 }
 
 TEST_F(FunctionsDataViewTest, InvalidColumnAndRowNumbersReturnEmptyString) {
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
   EXPECT_EQ(view_.GetValue(1, 0), "");    // Invalid row index
   EXPECT_EQ(view_.GetValue(0, 25), "");   // Invalid column index
@@ -179,6 +183,7 @@ TEST_F(FunctionsDataViewTest, ViewHandlesMultipleElements) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0], &functions_[1], &functions_[2]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 3);
 
   // We don't expect the view to be in any particular order at this point.
@@ -194,6 +199,7 @@ TEST_F(FunctionsDataViewTest, ClearFunctionsRemovesAllElements) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0], &functions_[1], &functions_[2]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 3);
 
   view_.ClearFunctions();
@@ -203,9 +209,11 @@ TEST_F(FunctionsDataViewTest, ClearFunctionsRemovesAllElements) {
 TEST_F(FunctionsDataViewTest, RemoveFunctionsOfModule) {
   view_.AddFunctions(
       {&functions_[0], &functions_[1], &functions_[2], &functions_[3], &functions_[4]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 5);
 
   view_.RemoveFunctionsOfModule(functions_[2].module_path());
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 4);
   EXPECT_THAT(
       (std::array{view_.GetValue(0, 1), view_.GetValue(1, 1), view_.GetValue(2, 1),
@@ -214,13 +222,15 @@ TEST_F(FunctionsDataViewTest, RemoveFunctionsOfModule) {
                                     functions_[3].pretty_name(), functions_[4].pretty_name()));
 
   view_.RemoveFunctionsOfModule(functions_[3].module_path());
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 3);
   EXPECT_THAT(
       (std::array{view_.GetValue(0, 1), view_.GetValue(1, 1), view_.GetValue(2, 1)}),
       testing::UnorderedElementsAre(functions_[0].pretty_name(), functions_[1].pretty_name(),
                                     functions_[4].pretty_name()));
 
   view_.RemoveFunctionsOfModule(functions_[3].module_path());  // Should do nothing.
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 3);
 }
 
@@ -243,6 +253,7 @@ TEST_F(FunctionsDataViewTest, FunctionSelectionAppearsInFirstColumn) {
   EXPECT_CALL(app_, HasCaptureData).WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
 
   function_selected = false;
@@ -277,6 +288,7 @@ TEST_F(FunctionsDataViewTest, FrameTrackSelectionAppearsInFirstColumn) {
   EXPECT_CALL(app_, HasCaptureData).WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
 
   function_selected = false;
@@ -353,6 +365,7 @@ TEST_F(FunctionsDataViewTest, FrameTrackSelectionAppearsInFirstColumnWhenACaptur
       .WillRepeatedly(testing::ReturnPointee(&frame_track_enabled));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
 
   frame_track_enabled = true;
@@ -368,6 +381,7 @@ TEST_F(FunctionsDataViewTest, FunctionSizeAppearsInThirdColumn) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
   EXPECT_EQ(view_.GetValue(0, 2), std::to_string(functions_[0].size()));
 }
@@ -378,13 +392,15 @@ TEST_F(FunctionsDataViewTest, ModuleColumnShowsFilenameOfModule) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
   EXPECT_EQ(view_.GetValue(0, 3),
             std::filesystem::path{functions_[0].module_path()}.filename().string());
 }
 
 TEST_F(FunctionsDataViewTest, AddressColumnShowsAddress) {
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
   ASSERT_EQ(view_.GetNumElements(), 1);
 
   // We expect the address to be in hex - indicated by "0x"
@@ -413,6 +429,7 @@ TEST_F(FunctionsDataViewTest, ContextMenuEntriesChangeOnFunctionState) {
       });
 
   view_.AddFunctions({&functions_[0], &functions_[1], &functions_[2]});
+  view_.OnDataChanged();
 
   auto verify_context_menu_action_availability = [&](absl::Span<const int> selected_indices) {
     FlattenContextMenu context_menu = FlattenContextMenuWithGroupingAndCheckOrder(
@@ -477,6 +494,7 @@ TEST_F(FunctionsDataViewTest, GenericDataExportFunctionShowCorrectData) {
       .WillRepeatedly(testing::Return(false));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
 
   FlattenContextMenu context_menu =
       FlattenContextMenuWithGroupingAndCheckOrder(view_.GetContextMenuWithGrouping(0, {0}));
@@ -533,6 +551,7 @@ TEST_F(FunctionsDataViewTest, ColumnSorting) {
   std::vector<FunctionInfo> functions = functions_;
   view_.AddFunctions(
       {&functions_[0], &functions_[1], &functions_[2], &functions_[3], &functions_[4]});
+  view_.OnDataChanged();
 
   const auto verify_correct_sorting = [&]() {
     // We won't check all columns because we control the test data and know that checking address
@@ -613,6 +632,7 @@ TEST_F(FunctionsDataViewTest, ContextMenuActionsCallCorrespondingFunctionsInAppI
   EXPECT_CALL(app_, HasCaptureData).WillRepeatedly(testing::Return(true));
 
   view_.AddFunctions({&functions_[0]});
+  view_.OnDataChanged();
 
   const auto match_function = [&](const FunctionInfo& function) {
     EXPECT_EQ(function.address(), functions_[0].address());
@@ -678,6 +698,7 @@ TEST_F(FunctionsDataViewTest, FilteringByFunctionName) {
 
   view_.AddFunctions(
       {&functions_[0], &functions_[1], &functions_[2], &functions_[3], &functions_[4]});
+  view_.OnDataChanged();
 
   // Filtering by an empty string should result in all functions listed -> No filtering.
   view_.OnFilter("");
@@ -733,6 +754,7 @@ TEST_F(FunctionsDataViewTest, FilteringByModuleName) {
 
   view_.AddFunctions(
       {&functions_[0], &functions_[1], &functions_[2], &functions_[3], &functions_[4]});
+  view_.OnDataChanged();
 
   // Only the filename is considered when filtering, so searching for the full file path results in
   // an empty search result.
@@ -767,6 +789,7 @@ TEST_F(FunctionsDataViewTest, FilteringByFunctionAndModuleName) {
 
   view_.AddFunctions(
       {&functions_[0], &functions_[1], &functions_[2], &functions_[3], &functions_[4]});
+  view_.OnDataChanged();
 
   // ffind is the name of the function while CapitalizedModule is the filename of its module.
   view_.OnFilter("ffind CapitalizedModule");

src/DataViews/ModulesDataView.cpp

@@ -50,6 +50,11 @@ const std::vector<DataView::Column>& ModulesDataView::GetColumns() {
   return kColumns;
 }
 
+void ModulesDataView::OnDataChanged() {
+  ORBIT_SCOPE_FUNCTION;
+  DoFilter();
+}
+
 std::string ModulesDataView::GetValue(int row, int col) {
   uint64_t start_address = indices_[row];
   const ModuleData* module = start_address_to_module_.at(start_address);
@@ -96,6 +101,7 @@ std::string ModulesDataView::GetToolTip(int row, int column) {
   }
 
 void ModulesDataView::DoSort() {
+  ORBIT_SCOPE_FUNCTION;
   bool ascending = sorting_orders_[sorting_column_] == SortingOrder::kAscending;
   std::function<bool(uint64_t, uint64_t)> sorter = nullptr;
 

src/DataViews/include/DataViews/DataViewType.h

@@ -16,9 +16,34 @@ enum class DataViewType {
   kSampling,
   kPresets,
   kTracepoints,
-  kAll,
+  kAll,  // kAll needs to be last.
 };
 
+inline const char* ToString(DataViewType type) {
+  switch (type) {
+    case DataViewType::kInvalid:
+      return "kInvalid";
+    case DataViewType::kFunctions:
+      return "kFunctions";
+    case DataViewType::kLiveFunctions:
+      return "kLiveFunctions";
+    case DataViewType::kCallstack:
+      return "kCallstack";
+    case DataViewType::kModules:
+      return "kModules";
+    case DataViewType::kSampling:
+      return "kSampling";
+    case DataViewType::kPresets:
+      return "kPresets";
+    case DataViewType::kTracepoints:
+      return "kTracepoints";
+    case DataViewType::kAll:
+      return "kAll";
+  }
+  ORBIT_UNREACHABLE();
+  return "";
+}
+
 }  // namespace orbit_data_views
 
 #endif  // DATA_VIEWS_DATA_VIEW_TYPE_H_

src/DataViews/include/DataViews/ModulesDataView.h

@@ -27,6 +27,8 @@ class ModulesDataView : public DataView {
  public:
   explicit ModulesDataView(AppInterface* app);
 
+  void OnDataChanged() override;
+
   const std::vector<Column>& GetColumns() override;
   int GetDefaultSortingColumn() override { return kColumnFileSize; }
   std::string GetValue(int row, int column) override;

src/ObjectUtils/PdbFileDia.cpp

@@ -143,6 +143,7 @@ ErrorMessageOr<void> ForEachSymbolWithSymTag(const enum SymTagEnum& sym_tag,
 }  // namespace
 
 ErrorMessageOr<orbit_grpc_protos::ModuleSymbols> PdbFileDia::LoadDebugSymbols() {
+  ORBIT_SCOPE_FUNCTION;
   ModuleSymbols module_symbols;
   absl::flat_hash_set<uint64_t> addresses_from_module_info_stream;