[Lldb-commits] [PATCH] D55472: Speadup memory regions handling and cleanup related code

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha updated this revision to Diff 178690.
tatyana-krasnukha added a comment.

Replace vector of unique pointers with vector of values, revert some API 
changes, remove MinidumpParser changes.
I will put overridden GetMemoryRegions in a separate patch.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55472/new/

https://reviews.llvm.org/D55472

Files:
  include/lldb/API/SBMemoryRegionInfo.h
  include/lldb/API/SBMemoryRegionInfoList.h
  include/lldb/Target/Process.h
  include/lldb/lldb-forward.h
  source/API/SBMemoryRegionInfo.cpp
  source/API/SBMemoryRegionInfoList.cpp
  source/API/SBProcess.cpp
  source/Target/Process.cpp


Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6030,7 +6030,7 @@
 }
 
 Status
-Process::GetMemoryRegions(std::vector<lldb::MemoryRegionInfoSP> &region_list) {
+Process::GetMemoryRegions(std::vector<lldb_private::MemoryRegionInfo> &region_list) {
 
   Status error;
 
@@ -6038,7 +6038,7 @@
 
   region_list.clear();
   do {
-    lldb::MemoryRegionInfoSP region_info(new lldb_private::MemoryRegionInfo());
+    lldb_private::MemoryRegionInfo region_info;
     error = GetMemoryRegionInfo(range_end, *region_info);
     // GetMemoryRegionInfo should only return an error if it is unimplemented.
     if (error.Fail()) {
@@ -6048,7 +6048,7 @@
 
     range_end = region_info->GetRange().GetRangeEnd();
     if (region_info->GetMapped() == MemoryRegionInfo::eYes) {
-      region_list.push_back(region_info);
+      region_list.push_back(std::move(region_info));
     }
   } while (range_end != LLDB_INVALID_ADDRESS);
 
Index: source/API/SBProcess.cpp
===================================================================
--- source/API/SBProcess.cpp
+++ source/API/SBProcess.cpp
@@ -1358,17 +1358,17 @@
                                SBMemoryRegionInfo &sb_region_info) {
   lldb::SBError sb_error;
   ProcessSP process_sp(GetSP());
-  MemoryRegionInfoSP region_info_sp =
-      std::make_shared<lldb_private::MemoryRegionInfo>();
   if (process_sp) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&process_sp->GetRunLock())) {
       std::lock_guard<std::recursive_mutex> guard(
           process_sp->GetTarget().GetAPIMutex());
+
+      MemoryRegionInfo region_info;
       sb_error.ref() =
-          process_sp->GetMemoryRegionInfo(load_addr, *region_info_sp);
+          process_sp->GetMemoryRegionInfo(load_addr, region_info);
       if (sb_error.Success()) {
-        sb_region_info.ref() = *region_info_sp;
+        sb_region_info.ref() = std::move(region_info);
       }
     } else {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
@@ -1385,35 +1385,31 @@
 }
 
 lldb::SBMemoryRegionInfoList SBProcess::GetMemoryRegions() {
-  lldb::SBError sb_error;
   lldb::SBMemoryRegionInfoList sb_region_list;
+
   ProcessSP process_sp(GetSP());
-  if (process_sp) {
-    Process::StopLocker stop_locker;
-    if (stop_locker.TryLock(&process_sp->GetRunLock())) {
-      std::lock_guard<std::recursive_mutex> guard(
-          process_sp->GetTarget().GetAPIMutex());
-      std::vector<MemoryRegionInfoSP> region_list;
-      sb_error.ref() = process_sp->GetMemoryRegions(region_list);
-      if (sb_error.Success()) {
-        std::vector<MemoryRegionInfoSP>::iterator end = region_list.end();
-        for (std::vector<MemoryRegionInfoSP>::iterator it = region_list.begin();
-             it != end; it++) {
-          SBMemoryRegionInfo sb_region_info(it->get());
-          sb_region_list.Append(sb_region_info);
-        }
-      }
-    } else {
-      Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
-      if (log)
-        log->Printf(
-            "SBProcess(%p)::GetMemoryRegionInfo() => error: process is running",
-            static_cast<void *>(process_sp.get()));
-      sb_error.SetErrorString("process is running");
+  if (!process_sp)
+    return sb_region_list;
+
+  Process::StopLocker stop_locker;
+  if (stop_locker.TryLock(&process_sp->GetRunLock())) {
+    std::lock_guard<std::recursive_mutex> guard(
+        process_sp->GetTarget().GetAPIMutex());
+
+    std::vector<MemoryRegionInfo> region_list;
+    if (process_sp->GetMemoryRegions(region_list).Success()) {
+      sb_region_list.Reserve(sb_region_list.GetSize() + region_list.size());
+      for (auto& region : region_list)
+        sb_region_list.Append(SBMemoryRegionInfo(std::move(region)));
     }
   } else {
-    sb_error.SetErrorString("SBProcess is invalid");
+    Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
+    if (log)
+      log->Printf(
+          "SBProcess(%p)::GetMemoryRegionInfo() => error: process is running",
+          static_cast<void *>(process_sp.get()));
   }
+
   return sb_region_list;
 }
 
Index: source/API/SBMemoryRegionInfoList.cpp
===================================================================
--- source/API/SBMemoryRegionInfoList.cpp
+++ source/API/SBMemoryRegionInfoList.cpp
@@ -32,20 +32,24 @@
     return *this;
   }
 
-  uint32_t GetSize() { return m_regions.size(); }
+  size_t GetSize() const { return m_regions.size(); }
+
+  void Reserve(size_t capacity) { return m_regions.reserve(capacity); }
 
   void Append(const lldb::SBMemoryRegionInfo &sb_region) {
     m_regions.push_back(sb_region);
   }
 
   void Append(const MemoryRegionInfoListImpl &list) {
-    for (auto val : list.m_regions)
+    Reserve(GetSize() + list.GetSize());
+
+    for (const auto &val : list.m_regions)
       Append(val);
   }
 
   void Clear() { m_regions.clear(); }
 
-  bool GetMemoryRegionInfoAtIndex(uint32_t index,
+  bool GetMemoryRegionInfoAtIndex(size_t index,
                                   SBMemoryRegionInfo &region_info) {
     if (index >= GetSize())
       return false;
@@ -78,6 +82,10 @@
   return m_opaque_ap->GetSize();
 }
 
+void SBMemoryRegionInfoList::Reserve(size_t capacity) {
+  m_opaque_ap->Reserve(capacity);
+}
+
 bool SBMemoryRegionInfoList::GetMemoryRegionAtIndex(
     uint32_t idx, SBMemoryRegionInfo &region_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
@@ -99,11 +107,12 @@
 
 void SBMemoryRegionInfoList::Clear() { m_opaque_ap->Clear(); }
 
-void SBMemoryRegionInfoList::Append(SBMemoryRegionInfo &sb_region) {
+void SBMemoryRegionInfoList::Append(const SBMemoryRegionInfo &sb_region) {
   m_opaque_ap->Append(sb_region);
 }
 
-void SBMemoryRegionInfoList::Append(SBMemoryRegionInfoList &sb_region_list) {
+void
+SBMemoryRegionInfoList::Append(const SBMemoryRegionInfoList &sb_region_list) {
   m_opaque_ap->Append(*sb_region_list);
 }
 
Index: source/API/SBMemoryRegionInfo.cpp
===================================================================
--- source/API/SBMemoryRegionInfo.cpp
+++ source/API/SBMemoryRegionInfo.cpp
@@ -20,11 +20,8 @@
 SBMemoryRegionInfo::SBMemoryRegionInfo()
     : m_opaque_ap(new MemoryRegionInfo()) {}
 
-SBMemoryRegionInfo::SBMemoryRegionInfo(const MemoryRegionInfo *lldb_object_ptr)
-    : m_opaque_ap(new MemoryRegionInfo()) {
-  if (lldb_object_ptr)
-    ref() = *lldb_object_ptr;
-}
+SBMemoryRegionInfo::SBMemoryRegionInfo(MemoryRegionInfo lldb_object)
+    : m_opaque_ap(new MemoryRegionInfo(std::move(lldb_object))) {}
 
 SBMemoryRegionInfo::SBMemoryRegionInfo(const SBMemoryRegionInfo &rhs)
     : m_opaque_ap(new MemoryRegionInfo()) {
Index: include/lldb/lldb-forward.h
===================================================================
--- include/lldb/lldb-forward.h
+++ include/lldb/lldb-forward.h
@@ -369,7 +369,6 @@
 typedef std::shared_ptr<lldb_private::Listener> ListenerSP;
 typedef std::weak_ptr<lldb_private::Listener> ListenerWP;
 typedef std::shared_ptr<lldb_private::MemoryHistory> MemoryHistorySP;
-typedef std::shared_ptr<lldb_private::MemoryRegionInfo> MemoryRegionInfoSP;
 typedef std::unique_ptr<lldb_private::MemoryRegionInfo> MemoryRegionInfoUP;
 typedef std::shared_ptr<lldb_private::Module> ModuleSP;
 typedef std::weak_ptr<lldb_private::Module> ModuleWP;
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -2081,7 +2081,7 @@
   ///     An error value.
   //------------------------------------------------------------------
   virtual Status
-  GetMemoryRegions(std::vector<lldb::MemoryRegionInfoSP> &region_list);
+  GetMemoryRegions(std::vector<lldb_private::MemoryRegionInfo> &region_list);
 
   virtual Status GetWatchpointSupportInfo(uint32_t &num) {
     Status error;
Index: include/lldb/API/SBMemoryRegionInfoList.h
===================================================================
--- include/lldb/API/SBMemoryRegionInfoList.h
+++ include/lldb/API/SBMemoryRegionInfoList.h
@@ -30,9 +30,11 @@
 
   bool GetMemoryRegionAtIndex(uint32_t idx, SBMemoryRegionInfo &region_info);
 
-  void Append(lldb::SBMemoryRegionInfo &region);
+  void Append(const lldb::SBMemoryRegionInfo &region);
 
-  void Append(lldb::SBMemoryRegionInfoList &region_list);
+  void Append(const lldb::SBMemoryRegionInfoList &region_list);
+
+  void Reserve(size_t capacity);
 
   void Clear();
 
Index: include/lldb/API/SBMemoryRegionInfo.h
===================================================================
--- include/lldb/API/SBMemoryRegionInfo.h
+++ include/lldb/API/SBMemoryRegionInfo.h
@@ -102,7 +102,7 @@
 
   const lldb_private::MemoryRegionInfo &ref() const;
 
-  SBMemoryRegionInfo(const lldb_private::MemoryRegionInfo *lldb_object_ptr);
+  SBMemoryRegionInfo(lldb_private::MemoryRegionInfo lldb_object);
 
   lldb::MemoryRegionInfoUP m_opaque_ap;
 };

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits