Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lldb/include/lldb/Target/Statistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions lldb/source/API/SBTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

error.SetError(process_sp->LoadCore());
if (error.Success())
sb_process.SetSP(process_sp);
Expand Down
6 changes: 5 additions & 1 deletion lldb/source/Commands/CommandObjectTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ElapsedTime loadCoreTime(
ElapsedTime load_core_time(

target_sp->GetStatistics().GetLoadCoreTime());
error = process_sp->LoadCore();
}

if (error.Fail()) {
result.AppendError(error.AsCString("unknown core file format"));
Expand Down
5 changes: 5 additions & 0 deletions lldb/source/Target/Statistics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.
Expand Down
28 changes: 28 additions & 0 deletions lldb/test/API/functionalities/stats_api/TestStatisticsAPI.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Test the SBAPI for GetStatistics()

import json

import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
Expand Down Expand Up @@ -54,6 +55,11 @@ def test_stats_api(self):
stats_json,
'Make sure the "frameVariable" key in in target.GetStatistics()["targets"][0]',
)
self.assertNotIn(
"loadCoreTime",
stats_json,
"LoadCoreTime should not be present in a live, non-coredump target",
)
expressionEvaluation = stats_json["expressionEvaluation"]
self.assertIn(
"successes",
Expand Down Expand Up @@ -157,3 +163,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)
Original file line number Diff line number Diff line change
@@ -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
...
Loading