[LLDB] Add load core time to target metrics #161581

Jlalond · 2025-10-01T20:34:52Z

This patch adds a load core time, right now we don't have much insight into the performance of load core, especially for large coredumps. To start collecting information on this I've added some minor instrumentation code to measure the two call sites of LoadCore.

I've also added a test to validate the new metric is output in statistics dump

llvmbot · 2025-10-01T20:35:29Z

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

This patch adds a load core time, right now we don't have much insight into the performance of load core, especially for large coredumps. To start collecting information on this I've added some minor instrumentation code to measure the two call sites of LoadCore.

I've also added a test to validate the new metric is output in statistics dump

Full diff: https://github.com/llvm/llvm-project/pull/161581.diff

6 Files Affected:

(modified) lldb/include/lldb/Target/Statistics.h (+2)
(modified) lldb/source/API/SBTarget.cpp (+1)
(modified) lldb/source/Commands/CommandObjectTarget.cpp (+5-1)
(modified) lldb/source/Target/Statistics.cpp (+5)
(modified) lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py (+23)
(added) lldb/test/API/functionalities/stats_api/arm64-minidump-build-ids.yaml (+19)

diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h
index d6983bb0b9d24..2653835206ec7 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -322,12 +322,14 @@ class TargetStats {
   void IncreaseSourceRealpathCompatibleCount(uint32_t count);
 
   StatsDuration &GetCreateTime() { return m_create_time; }
+  StatsDuration &GetLoadCoreTime() { return m_load_core_time; }
   StatsSuccessFail &GetExpressionStats() { return m_expr_eval; }
   StatsSuccessFail &GetFrameVariableStats() { return m_frame_var; }
   void Reset(Target &target);
 
 protected:
   StatsDuration m_create_time;
+  StatsDuration m_load_core_time;
   std::optional<StatsTimepoint> m_launch_or_attach_time;
   std::optional<StatsTimepoint> m_first_private_stop_time;
   std::optional<StatsTimepoint> m_first_public_stop_time;
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index eb56337de3c44..90ffe786c696c 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -255,6 +255,7 @@ SBProcess SBTarget::LoadCore(const char *core_file, lldb::SBError &error) {
     ProcessSP process_sp(target_sp->CreateProcess(
         target_sp->GetDebugger().GetListener(), "", &filespec, false));
     if (process_sp) {
+      ElapsedTime loadCoreTime(target_sp->GetStatistics().GetLoadCoreTime());
       error.SetError(process_sp->LoadCore());
       if (error.Success())
         sb_process.SetSP(process_sp);
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index 940be42d1b6e3..b5fc49d58c1eb 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -418,7 +418,11 @@ class CommandObjectTargetCreate : public CommandObjectParsed {
         if (process_sp) {
           // Seems weird that we Launch a core file, but that is what we
           // do!
-          error = process_sp->LoadCore();
+          {
+            ElapsedTime loadCoreTime(
+                target_sp->GetStatistics().GetLoadCoreTime());
+            error = process_sp->LoadCore();
+          }
 
           if (error.Fail()) {
             result.AppendError(error.AsCString("unknown core file format"));
diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp
index 8ad8d507268e2..f7311a8b24416 100644
--- a/lldb/source/Target/Statistics.cpp
+++ b/lldb/source/Target/Statistics.cpp
@@ -148,6 +148,11 @@ TargetStats::ToJSON(Target &target,
     target_metrics_json.try_emplace("targetCreateTime",
                                     m_create_time.get().count());
 
+    if (m_load_core_time.get().count() > 0) {
+      target_metrics_json.try_emplace("loadCoreTime",
+                                      m_load_core_time.get().count());
+    }
+
     json::Array breakpoints_array;
     double totalBreakpointResolveTime = 0.0;
     // Report both the normal breakpoint list and the internal breakpoint list.
diff --git a/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py b/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
index f06c9ae14bb7a..b638347b4f6f4 100644
--- a/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
+++ b/lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
@@ -1,6 +1,7 @@
 # Test the SBAPI for GetStatistics()
 
 import json
+
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -157,3 +158,25 @@ def test_command_stats_force(self):
         stats_force.GetAsJSON(stream_force)
         debug_stats_force = json.loads(stream_force.GetData())
         self.assertEqual(debug_stats_force["totalDebugInfoByteSize"], 445)
+
+    def test_core_load_time(self):
+        """
+        Test to see if the coredump path is included in statistics dump.
+        """
+        yaml_file = "arm64-minidump-build-ids.yaml"
+        src_dir = self.getSourceDir()
+        minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp")
+        self.yaml2obj(os.path.join(src_dir, yaml_file), minidump_path)
+        target = self.dbg.CreateTarget(None)
+        process = target.LoadCore(minidump_path)
+        self.assertTrue(process.IsValid())
+
+        stats_options = lldb.SBStatisticsOptions()
+        stats = target.GetStatistics(stats_options)
+        stream = lldb.SBStream()
+        stats.GetAsJSON(stream)
+        debug_stats = json.loads(stream.GetData())
+        self.assertTrue("targets" in debug_stats)
+        target_info = debug_stats["targets"][0]
+        self.assertTrue("loadCoreTime" in target_info)
+        self.assertTrue(float(target_info["loadCoreTime"]) > 0.0)
diff --git a/lldb/test/API/functionalities/stats_api/arm64-minidump-build-ids.yaml b/lldb/test/API/functionalities/stats_api/arm64-minidump-build-ids.yaml
new file mode 100644
index 0000000000000..4acbc409d8082
--- /dev/null
+++ b/lldb/test/API/functionalities/stats_api/arm64-minidump-build-ids.yaml
@@ -0,0 +1,19 @@
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  ARM
+    Platform ID:     Linux
+    CSD Version:     '15E216'
+    CPU:
+      CPUID:           0x00000000
+  - Type:            ModuleList
+    Modules:
+      - Base of Image:   0x0000000000001000
+        Size of Image:   0x00001000
+        Module Name:     '/tmp/a'
+        CodeView Record: 4C4570420102030405060708090A0B0C0D0E0F1011121314
+      - Base of Image:   0x0000000000001000
+        Size of Image:   0x00001000
+        Module Name:     '/tmp/b'
+        CodeView Record: 4C4570420A141E28323C46505A646E78828C96A0AAB4BEC8
+...

dmpots

LGTM. Added a few minor questions/comments.

dmpots · 2025-10-01T20:54:35Z

lldb/source/Target/Statistics.cpp

    target_metrics_json.try_emplace("targetCreateTime",
                                    m_create_time.get().count());

+    if (m_load_core_time.get().count() > 0) {


Do we need to conditionally add the "loadCoreTime"? Seems a value of 0 would be just as clear and could be nice to always have the key present.

I'd prefer to not clutter the output with added metrics. I know it's JSON and any automation reading it probably won't be bothered but for humans it might be weird to see 'loadCoreTime' in your live debugging stats.

lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py

JDevlieghere

Please use the LLDB naming conventions for the local variables.

JDevlieghere · 2025-10-01T22:38:59Z

lldb/source/Commands/CommandObjectTarget.cpp

          // do!
-          error = process_sp->LoadCore();
+          {
+            ElapsedTime loadCoreTime(


Suggested change

ElapsedTime loadCoreTime(

ElapsedTime load_core_time(

JDevlieghere · 2025-10-01T22:39:34Z

lldb/source/API/SBTarget.cpp

    ProcessSP process_sp(target_sp->CreateProcess(
        target_sp->GetDebugger().GetListener(), "", &filespec, false));
    if (process_sp) {
+      ElapsedTime loadCoreTime(target_sp->GetStatistics().GetLoadCoreTime());


Suggested change

ElapsedTime loadCoreTime(target_sp->GetStatistics().GetLoadCoreTime());

ElapsedTime load_core_time(target_sp->GetStatistics().GetLoadCoreTime());

Hey @JDevlieghere welcome back from vacation! I'll make sure to fix this, this one slipped through just because I context switched back and forth from json mode so many times. Apologies!

Thanks! And no worries, that's why we have post-commit review :-)

jeffreytan81 · 2025-10-02T02:36:01Z

lldb/source/API/SBTarget.cpp

    ProcessSP process_sp(target_sp->CreateProcess(
        target_sp->GetDebugger().GetListener(), "", &filespec, false));
    if (process_sp) {
+      ElapsedTime loadCoreTime(target_sp->GetStatistics().GetLoadCoreTime());


Instead of doing this in two places, why not putting this into Process::LoadCore() in case there is a 3rd code path invoking it in future.

Fixes some casing mistakes I added in #161581

This patch adds a load core time, right now we don't have much insight into the performance of load core, especially for large coredumps. To start collecting information on this I've added some minor instrumentation code to measure the two call sites of `LoadCore`. I've also added a test to validate the new metric is output in statistics dump

Fixes some casing mistakes I added in llvm#161581

Add new target metric to track time

e51ed29

Jlalond requested a review from JDevlieghere as a code owner October 1, 2025 20:34

llvmbot added the lldb label Oct 1, 2025

Jlalond requested review from dmpots and jeffreytan81 October 1, 2025 20:35

dmpots approved these changes Oct 1, 2025

View reviewed changes

Add negative test condition to the live test case

daf7bf3

Jlalond enabled auto-merge (squash) October 1, 2025 21:10

Jlalond merged commit b66dfa7 into llvm:main Oct 1, 2025
9 checks passed

Jlalond deleted the coredump-startup-metric branch October 1, 2025 22:25

JDevlieghere reviewed Oct 1, 2025

View reviewed changes

jeffreytan81 reviewed Oct 2, 2025

View reviewed changes

Jlalond mentioned this pull request Oct 2, 2025

[LLDB][NFC] Fix variable casing issue #161691

Merged

Jlalond added a commit that referenced this pull request Oct 2, 2025

[LLDB][NFC] Fix variable casing issue (#161691)

3c8c500

Fixes some casing mistakes I added in #161581

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025

[LLDB][NFC] Fix variable casing issue (llvm#161691)

19bb09d

Fixes some casing mistakes I added in llvm#161581

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLDB] Add load core time to target metrics #161581

[LLDB] Add load core time to target metrics #161581

Uh oh!

Jlalond commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

dmpots left a comment

Uh oh!

dmpots Oct 1, 2025

Uh oh!

Jlalond Oct 1, 2025

Uh oh!

Uh oh!

Uh oh!

JDevlieghere left a comment

Uh oh!

JDevlieghere Oct 1, 2025

Uh oh!

JDevlieghere Oct 1, 2025

Uh oh!

Jlalond Oct 1, 2025

Uh oh!

JDevlieghere Oct 1, 2025

Uh oh!

jeffreytan81 Oct 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	ElapsedTime loadCoreTime(target_sp->GetStatistics().GetLoadCoreTime());
	ElapsedTime load_core_time(target_sp->GetStatistics().GetLoadCoreTime());

[LLDB] Add load core time to target metrics #161581

[LLDB] Add load core time to target metrics #161581

Uh oh!

Conversation

Jlalond commented Oct 1, 2025

Uh oh!

llvmbot commented Oct 1, 2025

Uh oh!

dmpots left a comment

Choose a reason for hiding this comment

Uh oh!

dmpots Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

jeffreytan81 Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants