Add breakpoint resolving stats to each target.

[clayborg]

Summary

This patch adds breakpoints to each target's statistics so we can track how long it takes to resolve each breakpoint. It also includes the structured data for each breakpoint so the exact breakpoint details are logged to allow for reproduction of slow resolving breakpoints. Each target gets a new "breakpoints" array that contains breakpoint details. Each breakpoint has "details" which is the JSON representation of a serialized breakpoint resolver and filter, "id" which is the breakpoint ID, and "resolveTime" which is the time in seconds it took to resolve the breakpoint. A snippet of the new data is shown here:

"targets": [
  {
    "breakpoints": [
      {
        "details": {...},
        "id": 1,
        "resolveTime": 0.00039291599999999999
      },
      {
        "details": {...},
        "id": 2,
        "resolveTime": 0.00022679199999999999
      }
    ],
    "totalBreakpointResolveTime": 0.00061970799999999996
  }
]

This provides full details on exactly how breakpoints were set and how long it took to resolve them.

[wallace]

lldb/source/Breakpoint/Breakpoint.cpp

1120

err, i'm not a fan of this. Why don't you just make this return an expected and make the main command fail if the data couldn't be gathered? That will be better than concealing some possible parsing issues.

[clayborg]

Changes in this version:

• If we fail to get "details" from the structured data, we put the error string into the "details" dictionary with the error string.
• Added "numLocations" (int64_t) and "numResolvedLocations" (int64_t) and "internal" (bool) into each breakpoint's stats.
• Include the internal breakpoints in the "breakpoints" so we are tracking all breakpoints that are being set.
• Removed the top level "names" field because it is already contained in the "details".

[clayborg]

lldb/source/Breakpoint/Breakpoint.cpp

1120

How about I put the error string into the "details" as a key/value pair if this has an error?

[wallace]

sounds good to me!

[jingham]

Note, the breakpoint serialization for breakpoints only serializes the filter/resolver and the breakpoint specific options. It doesn't do anything with locations or their options. After all, you when you go to reset a breakpoint in another session (which is what this serialization is for) you have no way of knowing what locations you are going to find.

So if you want to record the locations (for instance to make sure that your recreation actually did the same work) then you will have to record the locations some other way. However, you do record the number of locations, which is probably a good enough signal that you aren't recreating what you thought you were. So maybe you don't need to worry about this.

[clayborg]

In D112587#3092120, @jingham wrote:

Note, the breakpoint serialization for breakpoints only serializes the filter/resolver and the breakpoint specific options. It doesn't do anything with locations or their options. After all, you when you go to reset a breakpoint in another session (which is what this serialization is for) you have no way of knowing what locations you are going to find.

Good to know!

So if you want to record the locations (for instance to make sure that your recreation actually did the same work) then you will have to record the locations some other way. However, you do record the number of locations, which is probably a good enough signal that you aren't recreating what you thought you were. So maybe you don't need to worry about this.

I am mainly concerned with the overall details of a breakpoint, and usually for the breakpoints that have no locations due to a source mapping error, or lack of debug information. Both stats are available (debug info size + breakpoint filter/resolver details) so that will help us figure out why things are not working for people. I had locations before, but I am not sure we need that information just yet so I will leave that for later if we do end up needing it for some reason.

[jingham]

In D112587#3092125, @clayborg wrote:

In D112587#3092120, @jingham wrote:

Note, the breakpoint serialization for breakpoints only serializes the filter/resolver and the breakpoint specific options. It doesn't do anything with locations or their options. After all, you when you go to reset a breakpoint in another session (which is what this serialization is for) you have no way of knowing what locations you are going to find.

Good to know!

So if you want to record the locations (for instance to make sure that your recreation actually did the same work) then you will have to record the locations some other way. However, you do record the number of locations, which is probably a good enough signal that you aren't recreating what you thought you were. So maybe you don't need to worry about this.

I am mainly concerned with the overall details of a breakpoint, and usually for the breakpoints that have no locations due to a source mapping error, or lack of debug information. Both stats are available (debug info size + breakpoint filter/resolver details) so that will help us figure out why things are not working for people. I had locations before, but I am not sure we need that information just yet so I will leave that for later if we do end up needing it for some reason.

This seems like a reasonable choice.

lldb/include/lldb/Breakpoint/Breakpoint.h

 #include "lldb/Breakpoint/BreakpointID.h"
 #include "lldb/Breakpoint/BreakpointLocationCollection.h"
 #include "lldb/Breakpoint/BreakpointLocationList.h"
 #include "lldb/Breakpoint/BreakpointName.h"
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Breakpoint/Stoppoint.h"
 #include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/SearchFilter.h"
+#include "lldb/Target/Statistics.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/StringList.h"
 #include "lldb/Utility/StructuredData.h"
 
 namespace lldb_private {
 
 /// \class Breakpoint Breakpoint.h "lldb/Breakpoint/Breakpoint.h" Class that
 /// manages logical breakpoint setting.
bool AllowDelete() const {
     return GetPermissions().GetAllowDelete();
   }
 
   // This one should only be used by Target to copy breakpoints from target to
   // target - primarily from the dummy target to prime new targets.
   static lldb::BreakpointSP CopyFromBreakpoint(lldb::TargetSP new_target,
       const Breakpoint &bp_to_copy_from);
 
+  /// Get statistics associated with this breakpoint in JSON format.
+  llvm::json::Value GetStatistics();
+
+  /// Get the time it took to resolve all locations in this breakpoint.
+  StatsDuration GetResolveTime() const { return m_resolve_time; }
+
 protected:
   friend class Target;
   // Protected Methods
 
   /// Constructors and Destructors
   /// Only the Target can make a breakpoint, and it owns the breakpoint
   /// lifespans. The constructor takes a filter and a resolver.  Up in Target
   /// there are convenience variants that make breakpoints for some common
private:
 
   /// Number of times this breakpoint has been hit. This is kept separately
   /// from the locations hit counts, since locations can go away when their
   /// backing library gets unloaded, and we would lose hit counts.
   StoppointHitCounter m_hit_counter;
 
   BreakpointName::Permissions m_permissions;
 
+  StatsDuration m_resolve_time{0.0};
+
   void SendBreakpointChangedEvent(lldb::BreakpointEventType eventKind);
 
   void SendBreakpointChangedEvent(BreakpointEventData *data);
 
   Breakpoint(const Breakpoint &) = delete;
   const Breakpoint &operator=(const Breakpoint &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_BREAKPOINT_BREAKPOINT_H

lldb/source/Breakpoint/Breakpoint.cpp

bool Breakpoint::InvokeCallback(StoppointCallbackContext *context,
   return m_options.InvokeCallback(context, GetID(), bp_loc_id);
 }
 
 BreakpointOptions &Breakpoint::GetOptions() { return m_options; }
 
 const BreakpointOptions &Breakpoint::GetOptions() const { return m_options; }
 
 void Breakpoint::ResolveBreakpoint() {
-  if (m_resolver_sp)
+  if (m_resolver_sp) {
+    ElapsedTime elapsed(m_resolve_time);
     m_resolver_sp->ResolveBreakpoint(*m_filter_sp);
   }
+}
 
 void Breakpoint::ResolveBreakpointInModules(
     ModuleList &module_list, BreakpointLocationCollection &new_locations) {
+  ElapsedTime elapsed(m_resolve_time);
   m_locations.StartRecordingNewLocations(new_locations);
 
   m_resolver_sp->ResolveBreakpointInModules(*m_filter_sp, module_list);
 
   m_locations.StopRecordingNewLocations();
 }
 
 void Breakpoint::ResolveBreakpointInModules(ModuleList &module_list,
if (!IsInternal() && send_event) {
           module_list, new_locations_event->GetBreakpointLocationCollection());
 
       if (new_locations_event->GetBreakpointLocationCollection().GetSize() !=
           0) {
         SendBreakpointChangedEvent(new_locations_event);
       } else
         delete new_locations_event;
     } else {
+      ElapsedTime elapsed(m_resolve_time);
       m_resolver_sp->ResolveBreakpointInModules(*m_filter_sp, module_list);
     }
   }
 }
 
 void Breakpoint::ClearAllBreakpointSites() {
   m_locations.ClearAllBreakpointSites();
 }
Breakpoint::BreakpointEventData::GetBreakpointLocationAtIndexFromEvent(
 
   const BreakpointEventData *data = GetEventDataFromEvent(event_sp.get());
   if (data) {
     bp_loc_sp = data->m_locations.GetByIndex(bp_loc_idx);
   }
 
   return bp_loc_sp;
 }
+
+json::Value Breakpoint::GetStatistics() {
+  json::Object bp;
+  bp.try_emplace("id", GetID());
+  bp.try_emplace("resolveTime", m_resolve_time.count());
+  bp.try_emplace("numLocations", (int64_t)GetNumLocations());
+  bp.try_emplace("numResolvedLocations", (int64_t)GetNumResolvedLocations());
+  bp.try_emplace("internal", IsInternal());
+  if (!m_kind_description.empty())
+    bp.try_emplace("kindDescription", m_kind_description);
+  // Put the full structured data for reproducing this breakpoint in a key/value
+  // pair named "details". This allows the breakpoint's details to be visible
+  // in the stats in case we need to reproduce a breakpoint that has long 
+  // resolve times
+  StructuredData::ObjectSP bp_data_sp = SerializeToStructuredData();
+  if (bp_data_sp) {
+    std::string buffer;
+    llvm::raw_string_ostream ss(buffer);
+    json::OStream json_os(ss);
+    bp_data_sp->Serialize(json_os);
+    if (auto expected_value = llvm::json::parse(ss.str())) {
+      bp.try_emplace("details", std::move(*expected_value));
+    } else {
+      std::string details_error = toString(expected_value.takeError());
+      json::Object details;
+      details.try_emplace("error", details_error);
+      bp.try_emplace("details", std::move(details));
+    }
+  }
+  return json::Value(std::move(bp));
+}

lldb/source/Target/Statistics.cpp

json::Value TargetStats::ToJSON(Target &target) {
   }
   if (m_launch_or_attach_time && m_first_public_stop_time) {
     double elapsed_time =
         elapsed(*m_launch_or_attach_time, *m_first_public_stop_time);
     target_metrics_json.try_emplace("firstStopTime", elapsed_time);
   }
   target_metrics_json.try_emplace("targetCreateTime", m_create_time.count());
 
+  json::Array breakpoints_array;
+  double totalBreakpointResolveTime = 0.0;
+  // Rport both the normal breakpoint list and the internal breakpoint list.
+  for (int i = 0; i < 2; ++i) {
+    BreakpointList &breakpoints = target.GetBreakpointList(i == 1);
+    std::unique_lock<std::recursive_mutex> lock;
+    breakpoints.GetListMutex(lock);
+    size_t num_breakpoints = breakpoints.GetSize();
+    for (size_t i = 0; i < num_breakpoints; i++) {
+      Breakpoint *bp = breakpoints.GetBreakpointAtIndex(i).get();
+      breakpoints_array.push_back(bp->GetStatistics());
+      totalBreakpointResolveTime += bp->GetResolveTime().count();
+    }
+  }
+
+  target_metrics_json.try_emplace("breakpoints", std::move(breakpoints_array));
+  target_metrics_json.try_emplace("totalBreakpointResolveTime",
+                                  totalBreakpointResolveTime);
+
   return target_metrics_json;
 }
 
 void TargetStats::SetLaunchOrAttachTime() {
   m_launch_or_attach_time = StatsClock::now();
   m_first_private_stop_time = llvm::None;
 }
 

lldb/test/API/commands/statistics/basic/TestStats.py

def test_modules(self):
             'path',
             'symbolTableIndexTime',
             'symbolTableParseTime',
             'triple',
             'uuid',
         ]
         self.assertNotEqual(exe_module, None)
         self.verify_keys(exe_module, 'module dict for "%s"' % (exe), module_keys)
+
+    def test_breakpoints(self):
+        """Test "statistics dump"
+
+        Output expected to be something like:
+
+        {
+          "modules" : [...],
+          "targets" : [
+                {
+                    "firstStopTime": 0.34164492800000001,
+                    "launchOrAttachTime": 0.31969605400000001,
+                    "moduleIdentifiers": [...],
+                    "targetCreateTime": 0.0040863039999999998
+                    "expressionEvaluation": {
+                        "failures": 0,
+                        "successes": 0
+                    },
+                    "frameVariable": {
+                        "failures": 0,
+                        "successes": 0
+                    },
+                    "breakpoints": [
+                        {
+                            "details": {...},
+                            "id": 1,
+                            "resolveTime": 2.65438675
+                        },
+                        {
+                            "details": {...},
+                            "id": 2,
+                            "resolveTime": 4.3632581669999997
+                        }                        
+                    ]
+                }
+            ],
+            "totalDebugInfoByteSize": 182522234,
+            "totalDebugInfoIndexTime": 2.33343,
+            "totalDebugInfoParseTime": 8.2121400240000071,
+            "totalSymbolTableParseTime": 0.123,
+            "totalSymbolTableIndexTime": 0.234,
+            "totalBreakpointResolveTime": 7.0176449170000001
+        }
+
+        """
+        target = self.createTestTarget()
+        self.runCmd("b main.cpp:7")
+        self.runCmd("b a_function")
+        debug_stats = self.get_stats()
+        debug_stat_keys = [
+            'modules', 
+            'targets',
+            'totalSymbolTableParseTime',
+            'totalSymbolTableIndexTime',
+            'totalDebugInfoParseTime',
+            'totalDebugInfoIndexTime',
+            'totalDebugInfoByteSize',
+        ]
+        self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
+        target_stats = debug_stats['targets'][0]
+        keys_exist = [
+            'breakpoints',
+            'expressionEvaluation',
+            'frameVariable',
+            'targetCreateTime',
+            'moduleIdentifiers',
+            'totalBreakpointResolveTime',
+        ]
+        self.verify_keys(target_stats, '"stats"', keys_exist, None)
+        self.assertGreater(target_stats['totalBreakpointResolveTime'], 0.0)
+        breakpoints = target_stats['breakpoints']
+        bp_keys_exist = [
+            'details',
+            'id',
+            'internal',
+            'numLocations',
+            'numResolvedLocations',
+            'resolveTime'
+        ]
+        for breakpoint in breakpoints:
+            self.verify_keys(breakpoint, 'target_stats["breakpoints"]', 
+                             bp_keys_exist, None)