Improve the performance of jModulesInfo in lldb-server

[tberghammer]

Summary

Improve the performance of jModulesInfo in lldb-server

Previously it parsed /proc/<pid>/maps for every module separately
resulting in a very slow response time. This CL add some caching and
optimizes the implementation to improve the code from O(n*m) to O(n+m)
where n is the number of modules requested and m is the number of
files mapped into memory.

[labath]

Looks great, thanks.

Do you have any measurements on the real impact this makes on jModulesInfo packet?

source/Plugins/Process/Linux/NativeProcessLinux.cpp

1757 ↗

(On Diff #82884)

Please put an early return here:
if (!cache.empty() { log_if_you_want_to(); return Error(); }

[tberghammer]

On some extremely large cases I seen the response time to fell from ~120s to ~8s (default packet timeout is 2s).

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h

GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
                                   size_t &actual_opcode_size,
                                   const uint8_t *&trap_opcode_bytes) override;
 
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
 
   LazyBool m_supports_mem_region;
-  std::vector<MemoryRegionInfo> m_mem_region_cache;
+  std::vector<std::pair<MemoryRegionInfo, FileSpec>> m_mem_region_cache;
 
   lldb::tid_t m_pending_notification_tid;
 
   // List of thread ids stepping with a breakpoint with the address of
   // the relevan breakpoint
   std::map<lldb::tid_t, lldb::addr_t> m_threads_stepping_with_breakpoint;
 
   // ---------------------------------------------------------------------
#endif
   // of resume
   // operation (continue, single-step) depends on the state parameter.
   Error ResumeThread(NativeThreadLinux &thread, lldb::StateType state,
                      int signo);
 
   void ThreadWasCreated(NativeThreadLinux &thread);
 
   void SigchldHandler();
+
+  Error PopulateMemoryRegionCache();
 };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeProcessLinux_H_

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

Error NativeProcessLinux::GetMemoryRegionInfo(lldb::addr_t load_addr,
   // FIXME review that the final memory region returned extends to the end of
   // the virtual address space,
   // with no perms if it is not mapped.
 
   // Use an approach that reads memory regions from /proc/{pid}/maps.
   // Assume proc maps entries are in ascending order.
   // FIXME assert if we find differently.
 
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-  Error error;
-
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
     // We're done.
-    error.SetErrorString("unsupported");
-    return error;
-  }
-
-  // If our cache is empty, pull the latest.  There should always be at least
-  // one memory region
-  // if memory region handling is supported.
-  if (m_mem_region_cache.empty()) {
-    error = ProcFileReader::ProcessLineByLine(
-        GetID(), "maps", [&](const std::string &line) -> bool {
-          MemoryRegionInfo info;
-          const Error parse_error =
-              ParseMemoryRegionInfoFromProcMapsLine(line, info);
-          if (parse_error.Success()) {
-            m_mem_region_cache.push_back(info);
-            return true;
-          } else {
-            if (log)
-              log->Printf("NativeProcessLinux::%s failed to parse proc maps "
-                          "line '%s': %s",
-                          __FUNCTION__, line.c_str(), error.AsCString());
-            return false;
-          }
-        });
+    return Error("unsupported");
 
-    // If we had an error, we'll mark unsupported.
-    if (error.Fail()) {
-      m_supports_mem_region = LazyBool::eLazyBoolNo;
-      return error;
-    } else if (m_mem_region_cache.empty()) {
-      // No entries after attempting to read them.  This shouldn't happen if
-      // /proc/{pid}/maps
-      // is supported.  Assume we don't support map entries via procfs.
-      if (log)
-        log->Printf("NativeProcessLinux::%s failed to find any procfs maps "
-                    "entries, assuming no support for memory region metadata "
-                    "retrieval",
-                    __FUNCTION__);
-      m_supports_mem_region = LazyBool::eLazyBoolNo;
-      error.SetErrorString("not supported");
-      return error;
-    }
+  Error error = PopulateMemoryRegionCache();
 
-    if (log)
-      log->Printf("NativeProcessLinux::%s read %" PRIu64
-                  " memory region entries from /proc/%" PRIu64 "/maps",
-                  __FUNCTION__,
-                  static_cast<uint64_t>(m_mem_region_cache.size()), GetID());
-
-    // We support memory retrieval, remember that.
-    m_supports_mem_region = LazyBool::eLazyBoolYes;
-  } else {
-    if (log)
-      log->Printf("NativeProcessLinux::%s reusing %" PRIu64
-                  " cached memory region entries",
-                  __FUNCTION__,
-                  static_cast<uint64_t>(m_mem_region_cache.size()));
-  }
-
   lldb::addr_t prev_base_address = 0;
 
   // FIXME start by finding the last region that is <= target address using
   // binary search.  Data is sorted.
   // There can be a ton of regions on pthreads apps with lots of threads.
   for (auto it = m_mem_region_cache.begin(); it != m_mem_region_cache.end();
        ++it) {
-    MemoryRegionInfo &proc_entry_info = *it;
+    MemoryRegionInfo &proc_entry_info = it->first;
 
     // Sanity check assumption that /proc/{pid}/maps entries are ascending.
     assert((proc_entry_info.GetRange().GetRangeBase() >= prev_base_address) &&
            "descending /proc/pid/maps entries detected, unexpected");
     prev_base_address = proc_entry_info.GetRange().GetRangeBase();
 
     // If the target address comes before this entry, indicate distance to next
     // region.
Error NativeProcessLinux::GetMemoryRegionInfo(lldb::addr_t load_addr,
   range_info.GetRange().SetRangeEnd(LLDB_INVALID_ADDRESS);
   range_info.SetReadable(MemoryRegionInfo::OptionalBool::eNo);
   range_info.SetWritable(MemoryRegionInfo::OptionalBool::eNo);
   range_info.SetExecutable(MemoryRegionInfo::OptionalBool::eNo);
   range_info.SetMapped(MemoryRegionInfo::OptionalBool::eNo);
   return error;
 }
 
+Error NativeProcessLinux::PopulateMemoryRegionCache() {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+  // If our cache is empty, pull the latest.  There should always be at least
+  // one memory region if memory region handling is supported.
+  if (!m_mem_region_cache.empty()) {
+    if (log)
+      log->Printf("NativeProcessLinux::%s reusing %" PRIu64
+                  " cached memory region entries",
+                  __FUNCTION__,
+                  static_cast<uint64_t>(m_mem_region_cache.size()));
+    return Error();
+  }
+
+  Error error = ProcFileReader::ProcessLineByLine(
+      GetID(), "maps", [&](const std::string &line) -> bool {
+        MemoryRegionInfo info;
+        const Error parse_error =
+            ParseMemoryRegionInfoFromProcMapsLine(line, info);
+        if (parse_error.Success()) {
+          m_mem_region_cache.emplace_back(
+              info, FileSpec(info.GetName().GetCString(), true));
+          return true;
+        } else {
+          if (log)
+            log->Printf("NativeProcessLinux::%s failed to parse proc maps "
+                        "line '%s': %s",
+                        __FUNCTION__, line.c_str(), parse_error.AsCString());
+          return false;
+        }
+      });
+
+  // If we had an error, we'll mark unsupported.
+  if (error.Fail()) {
+    m_supports_mem_region = LazyBool::eLazyBoolNo;
+    return error;
+  } else if (m_mem_region_cache.empty()) {
+    // No entries after attempting to read them.  This shouldn't happen if
+    // /proc/{pid}/maps is supported. Assume we don't support map entries
+    // via procfs.
+    if (log)
+      log->Printf("NativeProcessLinux::%s failed to find any procfs maps "
+                  "entries, assuming no support for memory region metadata "
+                  "retrieval",
+                  __FUNCTION__);
+    m_supports_mem_region = LazyBool::eLazyBoolNo;
+    error.SetErrorString("not supported");
+    return error;
+  }
+
+  if (log)
+    log->Printf("NativeProcessLinux::%s read %" PRIu64
+                " memory region entries from /proc/%" PRIu64 "/maps",
+                __FUNCTION__, static_cast<uint64_t>(m_mem_region_cache.size()),
+                GetID());
+
+  // We support memory retrieval, remember that.
+  m_supports_mem_region = LazyBool::eLazyBoolYes;
+  return Error();
+}
+
 void NativeProcessLinux::DoStopIDBumped(uint32_t newBumpId) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
   if (log)
     log->Printf("NativeProcessLinux::%s(newBumpId=%" PRIu32 ") called",
                 __FUNCTION__, newBumpId);
 
   if (log)
     log->Printf("NativeProcessLinux::%s clearing %" PRIu64
if (error.Fail()) {
     return error;
   }
 
   return error;
 }
 
 Error NativeProcessLinux::GetLoadedModuleFileSpec(const char *module_path,
                                                   FileSpec &file_spec) {
+  Error error = PopulateMemoryRegionCache();
+  if (error.Fail())
+    return error;
+
   FileSpec module_file_spec(module_path, true);
 
-  bool found = false;
   file_spec.Clear();
-  ProcFileReader::ProcessLineByLine(
-      GetID(), "maps", [&](const std::string &line) {
-        SmallVector<StringRef, 16> columns;
-        StringRef(line).split(columns, " ", -1, false);
-        if (columns.size() < 6)
-          return true; // continue searching
-
-        FileSpec this_file_spec(columns[5].str(), false);
-        if (this_file_spec.GetFilename() != module_file_spec.GetFilename())
-          return true; // continue searching
-
-        file_spec = this_file_spec;
-        found = true;
-        return false; // we are done
-      });
-
-  if (!found)
-    return Error("Module file (%s) not found in /proc/%" PRIu64 "/maps file!",
-                 module_file_spec.GetFilename().AsCString(), GetID());
-
-  return Error();
+  for (const auto &it : m_mem_region_cache) {
+    if (it.second.GetFilename() == module_file_spec.GetFilename()) {
+      file_spec = it.second;
+      return Error();
+    }
+  }
 }
 
 Error NativeProcessLinux::GetFileLoadAddress(const llvm::StringRef &file_name,
                                              lldb::addr_t &load_addr) {
   load_addr = LLDB_INVALID_ADDRESS;
-  Error error = ProcFileReader::ProcessLineByLine(
-      GetID(), "maps", [&](const std::string &line) -> bool {
-        StringRef maps_row(line);
-
-        SmallVector<StringRef, 16> maps_columns;
-        maps_row.split(maps_columns, StringRef(" "), -1, false);
+  Error error = PopulateMemoryRegionCache();
+  if (error.Fail())
+    return error;
 
-        if (maps_columns.size() < 6) {
-          // Return true to continue reading the proc file
-          return true;
-        }
-
-        if (maps_columns[5] == file_name) {
-          StringExtractor addr_extractor(maps_columns[0].str().c_str());
-          load_addr = addr_extractor.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
-
-          // Return false to stop reading the proc file further
-          return false;
-        }
-
-        // Return true to continue reading the proc file
-        return true;
-      });
-  return error;
+  FileSpec file(file_name, false);
+  for (const auto &it : m_mem_region_cache) {
+    if (it.second == file) {
+      load_addr = it.first.GetRange().GetRangeBase();
+      return Error();
+  return Error("No load address found for specified file.");
 }
 
 NativeThreadLinuxSP NativeProcessLinux::GetThreadByID(lldb::tid_t tid) {
   return std::static_pointer_cast<NativeThreadLinux>(
       NativeProcessProtocol::GetThreadByID(tid));
 }
 
 Error NativeProcessLinux::ResumeThread(NativeThreadLinux &thread,