[Lldb-commits] [PATCH] D150805: Proof of concept for reducing progress-reporting frequency.

[Sterling Augustine via Phabricator via lldb-commits]

saugustine created this revision.
Herald added subscribers: arphaman, kristof.beyls.
Herald added a project: All.
saugustine requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Reporting progress for every DIE read turns out to be very slow when
run over a remote connection such as ssh. We have a report of it
taking over 30-minutes to load the Dwarf for Chrome via ssh (which
transfers every single write) and only about a minute over
Chrome-Remote Desktop, which is a video-conferencing style link, and
so doesn't update nearly as often.

For a 7k DIE target, this improves the speed of reading on my personal
machine (entirely local) by about 3%; data below. Over remote, slower
connections the increase is likely much greater.

Top of trunk:
(lldb) target create "crash_test"
Current executable set to 'crash_test' (aarch64).
(lldb) log timers dump
12.509606661 sec (total: 12.510s; child: 0.000s; count: 7826) for void DWARFUnit::ExtractDIEsRWLocked()
...

With this change:
(lldb) target create "crash_test"
Current executable set to 'crash_test' (aarch64).
(lldb) log timers dump
12.139054862 sec (total: 12.139s; child: 0.000s; count: 7826) for void DWARFUnit::ExtractDIEsRWLocked()


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150805

Files:
  lldb/include/lldb/Core/Progress.h
  lldb/source/Core/Progress.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -76,7 +76,8 @@
   const uint64_t total_progress = units_to_index.size() * 2 + 8;
   Progress progress(
       llvm::formatv("Manually indexing DWARF for {0}", module_desc.GetData()),
-      total_progress);
+      total_progress,
+      /*report_increment=*/ 1000);
 
   std::vector<IndexSet> sets(units_to_index.size());
 
Index: lldb/source/Core/Progress.cpp
===================================================================
--- lldb/source/Core/Progress.cpp
+++ lldb/source/Core/Progress.cpp
@@ -16,9 +16,10 @@
 
 std::atomic<uint64_t> Progress::g_id(0);
 
-Progress::Progress(std::string title, uint64_t total,
+Progress::Progress(std::string title, uint64_t total, uint64_t report_increment,
                    lldb_private::Debugger *debugger)
-    : m_title(title), m_id(++g_id), m_completed(0), m_total(total) {
+    : m_title(title), m_id(++g_id), m_completed(0),
+      m_report_increment(report_increment), m_last_reported(0), m_total(total) {
   assert(total > 0);
   if (debugger)
     m_debugger_id = debugger->GetID();
@@ -54,7 +55,11 @@
     // Make sure we only send one notification that indicates the progress is
     // complete.
     m_complete = m_completed == m_total;
-    Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
-                             m_total, m_debugger_id);
+    if (m_complete || m_completed == 0 ||
+        m_completed >= m_last_reported + m_report_increment) {
+      m_last_reported = m_completed;
+      Debugger::ReportProgress(m_id, m_title, std::move(update), m_completed,
+                               m_total, m_debugger_id);
+    }
   }
 }
Index: lldb/include/lldb/Core/Progress.h
===================================================================
--- lldb/include/lldb/Core/Progress.h
+++ lldb/include/lldb/Core/Progress.h
@@ -67,9 +67,13 @@
   /// set to UINT64_MAX then an indeterminate progress indicator should be
   /// displayed.
   ///
+  /// @param [in] report_increment Notify only when progress has exceeded
+  /// this amount. Throttles messaging.
+  ///
   /// @param [in] debugger An optional debugger pointer to specify that this
   /// progress is to be reported only to specific debuggers.
   Progress(std::string title, uint64_t total = UINT64_MAX,
+           uint64_t report_increment = 1,
            lldb_private::Debugger *debugger = nullptr);
 
   /// Destroy the progress object.
@@ -101,6 +105,10 @@
   const uint64_t m_id;
   /// How much work ([0...m_total]) that has been completed.
   uint64_t m_completed;
+  /// Print a message when progress exceeds this amount.
+  uint64_t m_report_increment;
+  /// Progress at the time of last message.
+  uint64_t m_last_reported;
   /// Total amount of work, UINT64_MAX for non deterministic progress.
   const uint64_t m_total;
   /// The optional debugger ID to report progress to. If this has no value then


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150805.523118.patch
Type: text/x-patch
Size: 3125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230517/7f8faff7/attachment.bin>