[Lldb-commits] [lldb] b360dfd - [lldb] Create dependent modules in parallel (#114507)
Author: Jonas Devlieghere
Date: 2024-11-02T09:38:10-07:00
New Revision: b360dfd5031e76c97257ef1b3e90385bf297e8ab
URL:
https://github.com/llvm/llvm-project/commit/b360dfd5031e76c97257ef1b3e90385bf297e8ab
DIFF:
https://github.com/llvm/llvm-project/commit/b360dfd5031e76c97257ef1b3e90385bf297e8ab.diff
LOG: [lldb] Create dependent modules in parallel (#114507)
Create dependent modules in parallel in Target::SetExecutableModule.
This change was inspired by #110646 which takes the same approach when
attaching. Jason suggested we could use the same approach when you
create a target in LLDB.
I used Slack for benchmarking, which loads 902 images.
```
Benchmark 1: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 1.225 s ± 0.003 s[User: 3.977 s, System: 1.521 s]
Range (min … max):1.220 s … 1.229 s10 runs
Benchmark 2: ./bin/lldb /Applications/Slack.app/Contents/MacOS/Slack
Time (mean ± σ): 3.253 s ± 0.037 s[User: 3.013 s, System: 0.248 s]
Range (min … max):3.211 s … 3.310 s10 runs
```
We see about a 2x speedup, which matches what Jason saw for the attach
scenario. I also ran this under TSan to confirm this doesn't introduce
any races or deadlocks.
Added:
Modified:
lldb/source/Target/Target.cpp
Removed:
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 199efae8a728cc..8cd3fa8af6bae1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -68,6 +68,7 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SetVector.h"
+#include "llvm/Support/ThreadPool.h"
#include
#include
@@ -1575,7 +1576,6 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
m_arch.GetSpec().GetTriple().getTriple());
}
-FileSpecList dependent_files;
ObjectFile *executable_objfile = executable_sp->GetObjectFile();
bool load_dependents = true;
switch (load_dependent_files) {
@@ -1591,10 +1591,14 @@ void Target::SetExecutableModule(ModuleSP
&executable_sp,
}
if (executable_objfile && load_dependents) {
+ // FileSpecList is not thread safe and needs to be synchronized.
+ FileSpecList dependent_files;
+ std::mutex dependent_files_mutex;
+
+ // ModuleList is thread safe.
ModuleList added_modules;
- executable_objfile->GetDependentModules(dependent_files);
- for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
-FileSpec dependent_file_spec(dependent_files.GetFileSpecAtIndex(i));
+
+ auto GetDependentModules = [&](FileSpec dependent_file_spec) {
FileSpec platform_dependent_file_spec;
if (m_platform_sp)
m_platform_sp->GetFileWithUUID(dependent_file_spec, nullptr,
@@ -1608,9 +1612,48 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
-objfile->GetDependentModules(dependent_files);
+ if (objfile) {
+// Create a local copy of the dependent file list so we don't have
+// to lock for the whole duration of GetDependentModules.
+FileSpecList dependent_files_copy;
+{
+ std::lock_guard guard(dependent_files_mutex);
+ dependent_files_copy = dependent_files;
+}
+
+// Remember the size of the local copy so we can append only the
+// modules that have been added by GetDependentModules.
+const size_t previous_dependent_files =
+dependent_files_copy.GetSize();
+
+objfile->GetDependentModules(dependent_files_copy);
+
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (size_t i = previous_dependent_files;
+ i < dependent_files_copy.GetSize(); ++i)
+dependent_files.AppendIfUnique(
+dependent_files_copy.GetFileSpecAtIndex(i));
+}
+ }
+}
+ };
+
+ executable_objfile->GetDependentModules(dependent_files);
+
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
+// Process all currently known dependencies in parallel in the
innermost
+// loop. This may create newly discovered dependencies to be appended
to
+// dependent_files. We'll deal with these files during the next
+// iteration of the outermost loop.
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (; i < dependent_files.GetSize(); i++)
+task_group.async(GetDependentModules,
+ dependent_files.GetFileSpecAtIndex(i));
}
+task_group.wait();
[Lldb-commits] [lldb] [llvm] [DebugInfo] Add explicit visibility macros to CodeView template functions (PR #113102)
vgvassilev wrote: > The lldb change looks good (you can submit that in its own patch without a > review), but you'll probably want to find a different reviewer for the > codeview part (I have no idea who's responsible for that these days). Thank you, maybe @adrian-prantl will know? https://github.com/llvm/llvm-project/pull/113102 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/114507 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,48 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
-objfile->GetDependentModules(dependent_files);
+ if (objfile) {
+// Create a local copy of the dependent file list so we don't have
+// to lock for the whole duration of GetDependentModules.
+FileSpecList dependent_files_copy;
+{
+ std::lock_guard guard(dependent_files_mutex);
+ dependent_files_copy = dependent_files;
+}
+
+// Remember the size of the local copy so we can append only the
+// modules that have been added by GetDependentModules.
+const size_t previous_dependent_files =
+dependent_files_copy.GetSize();
+
+objfile->GetDependentModules(dependent_files_copy);
+
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (size_t i = previous_dependent_files;
+ i < dependent_files_copy.GetSize(); ++i)
+dependent_files.AppendIfUnique(
+dependent_files_copy.GetFileSpecAtIndex(i));
+}
+ }
+}
+ };
+
+ executable_objfile->GetDependentModules(dependent_files);
+
+ llvm::ThreadPoolTaskGroup task_group(Debugger::GetThreadPool());
+ for (uint32_t i = 0; i < dependent_files.GetSize(); i++) {
+// Process all currently known dependencies in parallel in the
innermost
+// loop. This may create newly discovered dependencies to be appended
to
+// dependent_files. We'll deal with these files during the next
+// iteration of the outermost loop.
+{
+ std::lock_guard guard(dependent_files_mutex);
+ for (; i < dependent_files.GetSize(); i++)
+task_group.async(GetDependentModules,
JDevlieghere wrote:
It doesn't, but if that becomes a problem we can do the same trick that Jason
suggested and iterate over a copy of the list here. For now that seems
unnecessary but hopefully someone will see this comment if that ever changes
:-)
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 79178ca - [lldb] Highlight "note:" in CommandReturnObject (#114610)
Author: Jonas Devlieghere
Date: 2024-11-02T08:52:32-07:00
New Revision: 79178ca689a8259d19d93320a6299e3d31383ac4
URL:
https://github.com/llvm/llvm-project/commit/79178ca689a8259d19d93320a6299e3d31383ac4
DIFF:
https://github.com/llvm/llvm-project/commit/79178ca689a8259d19d93320a6299e3d31383ac4.diff
LOG: [lldb] Highlight "note:" in CommandReturnObject (#114610)
We have helpers to emit warnings and errors. Do the same thing for notes
to they stand out more.
Added:
Modified:
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Interpreter/CommandReturnObject.cpp
Removed:
diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index a491a6c1535b11..9fef59337016df 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -110,6 +110,11 @@ class CommandReturnObject {
void AppendMessageWithFormat(const char *format, ...)
__attribute__((format(printf, 2, 3)));
+ void AppendNote(llvm::StringRef in_string);
+
+ void AppendNoteWithFormat(const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+
void AppendWarning(llvm::StringRef in_string);
void AppendWarningWithFormat(const char *format, ...)
@@ -127,6 +132,11 @@ class CommandReturnObject {
AppendMessage(llvm::formatv(format, std::forward(args)...).str());
}
+ template
+ void AppendNoteWithFormatv(const char *format, Args &&...args) {
+AppendNote(llvm::formatv(format, std::forward(args)...).str());
+ }
+
template
void AppendWarningWithFormatv(const char *format, Args &&... args) {
AppendWarning(llvm::formatv(format, std::forward(args)...).str());
diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..62c4e74d853ad1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -121,10 +121,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
if (note_shown)
return;
- result.GetOutputStream()
- << "note: object description requested, but type doesn't implement "
- "a custom object description. Consider using \"p\" instead of "
- "\"po\" (this note will only be shown once per debug session).\n";
+ result.AppendNote(
+ "object description requested, but type doesn't implement "
+ "a custom object description. Consider using \"p\" instead of "
+ "\"po\" (this note will only be shown once per debug session).\n");
note_shown = true;
}
};
@@ -164,8 +164,8 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgString();
-result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
-flags, expr);
+result.AppendNoteWithFormatv("ran `frame variable {0}{1}`", flags,
+ expr);
}
dump_val_object(*valobj_sp);
@@ -224,8 +224,7 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
StringRef flags;
if (args.HasArgs())
flags = args.GetArgStringWithDelimiter();
- result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
- expr);
+ result.AppendNoteWithFormatv("ran `expression {0}{1}`", flags, expr);
}
if (valobj_sp->GetError().GetError() != UserExpression::kNoResult)
diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 94f5ff608b2aea..2776efbb5ee36d 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -27,6 +27,12 @@ static llvm::raw_ostream &warning(Stream &strm) {
<< "warning: ";
}
+static llvm::raw_ostream ¬e(Stream &strm) {
+ return llvm::WithColor(strm.AsRawOstream(), llvm::HighlightColor::Note,
+ llvm::ColorMode::Enable)
+ << "note: ";
+}
+
static void DumpStringToStreamWithNewline(Stream &strm, const std::string &s) {
bool add_newline = false;
if (!s.empty()) {
@@ -74,6 +80,18 @@ void CommandReturnObject::AppendMessageWithFormat(const char
*format, ...) {
GetOutputStream() << sstrm.GetString();
}
+void CommandReturnObject::AppendNoteWithFormat(const char *format, ...) {
+ if (!format)
+return;
+ va_list args;
+ va_start(args, format);
+ StreamString sstrm;
+ sstrm.PrintfVarArg(format, args);
+ va_end(args);
+
+ note(GetOutputStream()) << sstrm.GetString();
+}
+
void CommandReturnObject::AppendWarningWithFormat(const char *format, ...) {
if (!format)
retur
[Lldb-commits] [lldb] [lldb] Highlight "note:" in CommandReturnObject (PR #114610)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/114610 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Create dependent modules in parallel (PR #114507)
@@ -1608,9 +1612,28 @@ void Target::SetExecutableModule(ModuleSP &executable_sp,
if (image_module_sp) {
added_modules.AppendIfNeeded(image_module_sp, false);
ObjectFile *objfile = image_module_sp->GetObjectFile();
- if (objfile)
+ if (objfile) {
+std::lock_guard guard(dependent_files_mutex);
objfile->GetDependentModules(dependent_files);
JDevlieghere wrote:
Yeah I did. I was hoping for a slight speedup, but the difference was in the
noise.
https://github.com/llvm/llvm-project/pull/114507
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
