This revision was automatically updated to reflect the committed changes.
Closed by commit rG51944e78bb37: [lldb] Add two-level caching in the source
manager (authored by JDevlieghere).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153834/new/
https://reviews.llvm.org/D153834
Files:
lldb/include/lldb/Core/SourceManager.h
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Utility/LLDBLog.h
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Utility/LLDBLog.cpp
lldb/test/API/source-manager/TestSourceManager.py
lldb/unittests/Core/SourceManagerTest.cpp
Index: lldb/unittests/Core/SourceManagerTest.cpp
===================================================================
--- lldb/unittests/Core/SourceManagerTest.cpp
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -30,7 +30,7 @@
// Insert: foo
FileSpec foo_file_spec("foo");
auto foo_file_sp =
- std::make_shared<SourceManager::File>(foo_file_spec, nullptr);
+ std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: foo, expect found.
@@ -44,7 +44,7 @@
// Insert: foo
FileSpec foo_file_spec("foo");
auto foo_file_sp =
- std::make_shared<SourceManager::File>(foo_file_spec, nullptr);
+ std::make_shared<SourceManager::File>(foo_file_spec, lldb::DebuggerSP());
cache.AddSourceFile(foo_file_spec, foo_file_sp);
// Query: bar, expect not found.
@@ -62,8 +62,8 @@
FileSystem::Instance().Resolve(resolved_foo_file_spec);
// Create the file with the resolved file spec.
- auto foo_file_sp =
- std::make_shared<SourceManager::File>(resolved_foo_file_spec, nullptr);
+ auto foo_file_sp = std::make_shared<SourceManager::File>(
+ resolved_foo_file_spec, lldb::DebuggerSP());
// Cache the result with the unresolved file spec.
cache.AddSourceFile(foo_file_spec, foo_file_sp);
Index: lldb/test/API/source-manager/TestSourceManager.py
===================================================================
--- lldb/test/API/source-manager/TestSourceManager.py
+++ lldb/test/API/source-manager/TestSourceManager.py
@@ -10,6 +10,7 @@
"""
import os
+import io
import stat
import lldb
@@ -37,6 +38,33 @@
self.file = self.getBuildArtifact("main-copy.c")
self.line = line_number("main.c", "// Set break point at this line.")
+ def modify_content(self):
+
+ # Read the main.c file content.
+ with io.open(self.file, "r", newline="\n") as f:
+ original_content = f.read()
+ if self.TraceOn():
+ print("original content:", original_content)
+
+ # Modify the in-memory copy of the original source code.
+ new_content = original_content.replace("Hello world", "Hello lldb", 1)
+
+ # Modify the source code file.
+ # If the source was read only, the copy will also be read only.
+ # Run "chmod u+w" on it first so we can modify it.
+ statinfo = os.stat(self.file)
+ os.chmod(self.file, statinfo.st_mode | stat.S_IWUSR)
+
+ with io.open(self.file, "w", newline="\n") as f:
+ time.sleep(1)
+ f.write(new_content)
+ if self.TraceOn():
+ print("new content:", new_content)
+ print(
+ "os.path.getmtime() after writing new content:",
+ os.path.getmtime(self.file),
+ )
+
def get_expected_stop_column_number(self):
"""Return the 1-based column number of the first non-whitespace
character in the breakpoint source line."""
@@ -234,32 +262,20 @@
self.fail("Fail to display source level breakpoints")
self.assertTrue(int(m.group(1)) > 0)
- # Read the main.c file content.
- with io.open(self.file, "r", newline="\n") as f:
- original_content = f.read()
- if self.TraceOn():
- print("original content:", original_content)
-
- # Modify the in-memory copy of the original source code.
- new_content = original_content.replace("Hello world", "Hello lldb", 1)
+ # Modify content
+ self.modify_content()
- # Modify the source code file.
- # If the source was read only, the copy will also be read only.
- # Run "chmod u+w" on it first so we can modify it.
- statinfo = os.stat(self.file)
- os.chmod(self.file, statinfo.st_mode | stat.S_IWUSR)
+ # Display the source code again. We should not see the updated line.
+ self.expect(
+ "source list -f main-copy.c -l %d" % self.line,
+ SOURCE_DISPLAYED_CORRECTLY,
+ substrs=["Hello world"],
+ )
- with io.open(self.file, "w", newline="\n") as f:
- time.sleep(1)
- f.write(new_content)
- if self.TraceOn():
- print("new content:", new_content)
- print(
- "os.path.getmtime() after writing new content:",
- os.path.getmtime(self.file),
- )
+ # clear the source cache.
+ self.runCmd("source cache clear")
- # Display the source code again. We should see the updated line.
+ # Display the source code again. Now we should see the updated line.
self.expect(
"source list -f main-copy.c -l %d" % self.line,
SOURCE_DISPLAYED_CORRECTLY,
@@ -338,3 +354,45 @@
# Make sure the main source file is no longer in the source cache.
self.expect("source cache dump", matching=False, substrs=[self.file])
+
+ def test_source_cache_interactions(self):
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+
+ # Create a first target.
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+ lldbutil.run_break_set_by_symbol(
+ self, "main", num_expected_locations=1
+ )
+ self.expect("run", RUN_SUCCEEDED, substrs=["Hello world"])
+
+ # Create a second target.
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+ lldbutil.run_break_set_by_symbol(
+ self, "main", num_expected_locations=1
+ )
+ self.expect("run", RUN_SUCCEEDED, substrs=["Hello world"])
+
+ # Modify the source file content.
+ self.modify_content()
+
+ # Clear the source cache. This will wipe the debugger and the process
+ # cache for the second process.
+ self.runCmd("source cache clear")
+
+ # Make sure we're seeing the new content from the clean process cache.
+ self.expect("next",
+ SOURCE_DISPLAYED_CORRECTLY,
+ substrs=["Hello lldb"],
+ )
+
+ # Switch back to the first target.
+ self.runCmd("target select 0")
+
+ # Make sure we're seeing the old content from the first target's
+ # process cache.
+ self.expect("next",
+ SOURCE_DISPLAYED_CORRECTLY,
+ substrs=["Hello world"],
+ )
+
Index: lldb/source/Utility/LLDBLog.cpp
===================================================================
--- lldb/source/Utility/LLDBLog.cpp
+++ lldb/source/Utility/LLDBLog.cpp
@@ -63,6 +63,7 @@
{{"on-demand"},
{"log symbol on-demand related activities"},
LLDBLog::OnDemand},
+ {{"source"}, {"log source related activities"}, LLDBLog::Source},
};
static Log::Channel g_log_channel(g_categories,
Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -21,10 +21,13 @@
#include "lldb/Symbol/LineEntry.h"
#include "lldb/Symbol/SymbolContext.h"
#include "lldb/Target/PathMappingList.h"
+#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/AnsiTerminal.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/DataBuffer.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
#include "lldb/Utility/RegularExpression.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-enumerations.h"
@@ -73,36 +76,89 @@
SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
if (!file_spec)
- return nullptr;
+ return {};
- DebuggerSP debugger_sp(m_debugger_wp.lock());
- FileSP file_sp;
- if (debugger_sp && debugger_sp->GetUseSourceCache())
- file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
+ Log *log = GetLog(LLDBLog::Source);
+ DebuggerSP debugger_sp(m_debugger_wp.lock());
TargetSP target_sp(m_target_wp.lock());
- // It the target source path map has been updated, get this file again so we
- // can successfully remap the source file
- if (target_sp && file_sp &&
- file_sp->GetSourceMapModificationID() !=
- target_sp->GetSourcePathMap().GetModificationID())
- file_sp.reset();
+ if (!debugger_sp || !debugger_sp->GetUseSourceCache()) {
+ LLDB_LOG(log, "Source file caching disabled: creating new source file: {0}",
+ file_spec);
+ if (target_sp)
+ return std::make_shared<File>(file_spec, target_sp);
+ return std::make_shared<File>(file_spec, debugger_sp);
+ }
+
+ ProcessSP process_sp = target_sp ? target_sp->GetProcessSP() : ProcessSP();
+
+ // Check the process source cache first. This is the fast path which avoids
+ // touching the file system unless the path remapping has changed.
+ if (process_sp) {
+ if (FileSP file_sp =
+ process_sp->GetSourceFileCache().FindSourceFile(file_spec)) {
+ LLDB_LOG(log, "Found source file in the process cache: {0}", file_spec);
+ if (file_sp->PathRemappingIsStale()) {
+ LLDB_LOG(log, "Path remapping is stale: removing file from caches: {0}",
+ file_spec);
+
+ // Remove the file from the debugger and process cache. Otherwise we'll
+ // hit the same issue again below when querying the debugger cache.
+ debugger_sp->GetSourceFileCache().RemoveSourceFile(file_sp);
+ process_sp->GetSourceFileCache().RemoveSourceFile(file_sp);
+
+ file_sp.reset();
+ } else {
+ return file_sp;
+ }
+ }
+ }
- // Update the file contents if needed if we found a file
+ // Cache miss in the process cache. Check the debugger source cache.
+ FileSP file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
+
+ // We found the file in the debugger cache. Check if anything invalidated our
+ // cache result.
if (file_sp)
- file_sp->UpdateIfNeeded();
+ LLDB_LOG(log, "Found source file in the debugger cache: {0}", file_spec);
+
+ // Check if the path remapping has changed.
+ if (file_sp && file_sp->PathRemappingIsStale()) {
+ LLDB_LOG(log, "Path remapping is stale: {0}", file_spec);
+ file_sp.reset();
+ }
+
+ // Check if the modification time has changed.
+ if (file_sp && file_sp->ModificationTimeIsStale()) {
+ LLDB_LOG(log, "Modification time is stale: {0}", file_spec);
+ file_sp.reset();
+ }
+
+ // Check if the file exists on disk.
+ if (file_sp && !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
+ LLDB_LOG(log, "File doesn't exist on disk: {0}", file_spec);
+ file_sp.reset();
+ }
- // If file_sp is no good or it points to a non-existent file, reset it.
- if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
+ // If at this point we don't have a valid file, it means we either didn't find
+ // it in the debugger cache or something caused it to be invalidated.
+ if (!file_sp) {
+ LLDB_LOG(log, "Creating and caching new source file: {0}", file_spec);
+
+ // (Re)create the file.
if (target_sp)
- file_sp = std::make_shared<File>(file_spec, target_sp.get());
+ file_sp = std::make_shared<File>(file_spec, target_sp);
else
file_sp = std::make_shared<File>(file_spec, debugger_sp);
- if (debugger_sp && debugger_sp->GetUseSourceCache())
- debugger_sp->GetSourceFileCache().AddSourceFile(file_spec, file_sp);
+ // Add the file to the debugger and process cache. If the file was
+ // invalidated, this will overwrite it.
+ debugger_sp->GetSourceFileCache().AddSourceFile(file_spec, file_sp);
+ if (process_sp)
+ process_sp->GetSourceFileCache().AddSourceFile(file_spec, file_sp);
}
+
return file_sp;
}
@@ -391,33 +447,36 @@
SourceManager::File::File(const FileSpec &file_spec,
lldb::DebuggerSP debugger_sp)
: m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
- m_debugger_wp(debugger_sp) {
- CommonInitializer(file_spec, nullptr);
+ m_debugger_wp(debugger_sp), m_target_wp(TargetSP()) {
+ CommonInitializer(file_spec, {});
}
-SourceManager::File::File(const FileSpec &file_spec, Target *target)
+SourceManager::File::File(const FileSpec &file_spec, TargetSP target_sp)
: m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(),
- m_debugger_wp(target ? target->GetDebugger().shared_from_this()
- : DebuggerSP()) {
- CommonInitializer(file_spec, target);
+ m_debugger_wp(target_sp ? target_sp->GetDebugger().shared_from_this()
+ : DebuggerSP()),
+ m_target_wp(target_sp) {
+ CommonInitializer(file_spec, target_sp);
}
void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
- Target *target) {
+ TargetSP target_sp) {
// Set the file and update the modification time.
SetFileSpec(file_spec);
+ // Always update the source map modification ID if we have a target.
+ if (target_sp)
+ m_source_map_mod_id = target_sp->GetSourcePathMap().GetModificationID();
+
// File doesn't exist.
if (m_mod_time == llvm::sys::TimePoint<>()) {
- if (target) {
- m_source_map_mod_id = target->GetSourcePathMap().GetModificationID();
-
+ if (target_sp) {
// If this is just a file name, try finding it in the target.
if (!file_spec.GetDirectory() && file_spec.GetFilename()) {
bool check_inlines = false;
SymbolContextList sc_list;
size_t num_matches =
- target->GetImages().ResolveSymbolContextForFilePath(
+ target_sp->GetImages().ResolveSymbolContextForFilePath(
file_spec.GetFilename().AsCString(), 0, check_inlines,
SymbolContextItem(eSymbolContextModule |
eSymbolContextCompUnit),
@@ -451,10 +510,10 @@
// Check target specific source remappings (i.e., the
// target.source-map setting), then fall back to the module
// specific remapping (i.e., the .dSYM remapping dictionary).
- auto remapped = target->GetSourcePathMap().FindFile(m_file_spec);
+ auto remapped = target_sp->GetSourcePathMap().FindFile(m_file_spec);
if (!remapped) {
FileSpec new_spec;
- if (target->GetImages().FindSourceFile(m_file_spec, new_spec))
+ if (target_sp->GetImages().FindSourceFile(m_file_spec, new_spec))
remapped = new_spec;
}
if (remapped)
@@ -540,18 +599,20 @@
return false;
}
-void SourceManager::File::UpdateIfNeeded() {
+bool SourceManager::File::ModificationTimeIsStale() const {
// TODO: use host API to sign up for file modifications to anything in our
// source cache and only update when we determine a file has been updated.
// For now we check each time we want to display info for the file.
auto curr_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
+ return curr_mod_time != llvm::sys::TimePoint<>() &&
+ m_mod_time != curr_mod_time;
+}
- if (curr_mod_time != llvm::sys::TimePoint<>() &&
- m_mod_time != curr_mod_time) {
- m_mod_time = curr_mod_time;
- m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec);
- m_offsets.clear();
- }
+bool SourceManager::File::PathRemappingIsStale() const {
+ if (TargetSP target_sp = m_target_wp.lock())
+ return GetSourceMapModificationID() !=
+ target_sp->GetSourcePathMap().GetModificationID();
+ return false;
}
size_t SourceManager::File::DisplaySourceLines(uint32_t line,
@@ -712,6 +773,8 @@
void SourceManager::SourceFileCache::AddSourceFile(const FileSpec &file_spec,
FileSP file_sp) {
+ llvm::sys::ScopedWriter guard(m_mutex);
+
assert(file_sp && "invalid FileSP");
AddSourceFileImpl(file_spec, file_sp);
@@ -720,6 +783,22 @@
AddSourceFileImpl(file_sp->GetFileSpec(), file_sp);
}
+void SourceManager::SourceFileCache::RemoveSourceFile(const FileSP &file_sp) {
+ llvm::sys::ScopedWriter guard(m_mutex);
+
+ assert(file_sp && "invalid FileSP");
+
+ // Iterate over all the elements in the cache.
+ // This is expensive but a relatively uncommon operation.
+ auto it = m_file_cache.begin();
+ while (it != m_file_cache.end()) {
+ if (it->second == file_sp)
+ it = m_file_cache.erase(it);
+ else
+ it++;
+ }
+}
+
void SourceManager::SourceFileCache::AddSourceFileImpl(
const FileSpec &file_spec, FileSP file_sp) {
FileCache::iterator pos = m_file_cache.find(file_spec);
@@ -733,11 +812,12 @@
SourceManager::FileSP SourceManager::SourceFileCache::FindSourceFile(
const FileSpec &file_spec) const {
- FileSP file_sp;
+ llvm::sys::ScopedReader guard(m_mutex);
+
FileCache::const_iterator pos = m_file_cache.find(file_spec);
if (pos != m_file_cache.end())
- file_sp = pos->second;
- return file_sp;
+ return pos->second;
+ return {};
}
void SourceManager::SourceFileCache::Dump(Stream &stream) const {
Index: lldb/source/Commands/CommandObjectSource.cpp
===================================================================
--- lldb/source/Commands/CommandObjectSource.cpp
+++ lldb/source/Commands/CommandObjectSource.cpp
@@ -22,6 +22,7 @@
#include "lldb/Symbol/CompileUnit.h"
#include "lldb/Symbol/Function.h"
#include "lldb/Symbol/Symbol.h"
+#include "lldb/Target/Process.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/StackFrame.h"
#include "lldb/Utility/FileSpec.h"
@@ -1213,8 +1214,18 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
+ // Dump the debugger source cache.
+ result.GetOutputStream() << "Debugger Source File Cache\n";
SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
cache.Dump(result.GetOutputStream());
+
+ // Dump the process source cache.
+ if (ProcessSP process_sp = m_exe_ctx.GetProcessSP()) {
+ result.GetOutputStream() << "\nProcess Source File Cache\n";
+ SourceManager::SourceFileCache &cache = process_sp->GetSourceFileCache();
+ cache.Dump(result.GetOutputStream());
+ }
+
result.SetStatus(eReturnStatusSuccessFinishResult);
return result.Succeeded();
}
@@ -1230,8 +1241,14 @@
protected:
bool DoExecute(Args &command, CommandReturnObject &result) override {
+ // Clear the debugger cache.
SourceManager::SourceFileCache &cache = GetDebugger().GetSourceFileCache();
cache.Clear();
+
+ // Clear the process cache.
+ if (ProcessSP process_sp = m_exe_ctx.GetProcessSP())
+ process_sp->GetSourceFileCache().Clear();
+
result.SetStatus(eReturnStatusSuccessFinishNoResult);
return result.Succeeded();
}
Index: lldb/include/lldb/Utility/LLDBLog.h
===================================================================
--- lldb/include/lldb/Utility/LLDBLog.h
+++ lldb/include/lldb/Utility/LLDBLog.h
@@ -48,6 +48,7 @@
Unwind = Log::ChannelFlag<29>,
Watchpoints = Log::ChannelFlag<30>,
OnDemand = Log::ChannelFlag<31>,
+ Source = Log::ChannelFlag<32>,
LLVM_MARK_AS_BITMASK_ENUM(OnDemand),
};
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -25,6 +25,7 @@
#include "lldb/Breakpoint/BreakpointSiteList.h"
#include "lldb/Core/LoadedModuleInfoList.h"
#include "lldb/Core/PluginInterface.h"
+#include "lldb/Core/SourceManager.h"
#include "lldb/Core/ThreadSafeValue.h"
#include "lldb/Core/ThreadedCommunication.h"
#include "lldb/Core/UserSettingsController.h"
@@ -2573,6 +2574,10 @@
virtual void ForceScriptedState(lldb::StateType state) {}
+ SourceManager::SourceFileCache &GetSourceFileCache() {
+ return m_source_file_cache;
+ }
+
protected:
friend class Trace;
@@ -3043,6 +3048,9 @@
std::unique_ptr<UtilityFunction> m_dlopen_utility_func_up;
llvm::once_flag m_dlopen_utility_func_flag_once;
+ /// Per process source file cache.
+ SourceManager::SourceFileCache m_source_file_cache;
+
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
Index: lldb/include/lldb/Core/SourceManager.h
===================================================================
--- lldb/include/lldb/Core/SourceManager.h
+++ lldb/include/lldb/Core/SourceManager.h
@@ -14,6 +14,7 @@
#include "lldb/lldb-forward.h"
#include "llvm/Support/Chrono.h"
+#include "llvm/Support/RWMutex.h"
#include <cstddef>
#include <cstdint>
@@ -36,11 +37,11 @@
const SourceManager::File &rhs);
public:
- File(const FileSpec &file_spec, Target *target);
+ File(const FileSpec &file_spec, lldb::TargetSP target_sp);
File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp);
- ~File() = default;
- void UpdateIfNeeded();
+ bool ModificationTimeIsStale() const;
+ bool PathRemappingIsStale() const;
size_t DisplaySourceLines(uint32_t line, std::optional<size_t> column,
uint32_t context_before, uint32_t context_after,
@@ -89,22 +90,29 @@
typedef std::vector<uint32_t> LineOffsets;
LineOffsets m_offsets;
lldb::DebuggerWP m_debugger_wp;
+ lldb::TargetWP m_target_wp;
private:
- void CommonInitializer(const FileSpec &file_spec, Target *target);
+ void CommonInitializer(const FileSpec &file_spec, lldb::TargetSP target_sp);
};
typedef std::shared_ptr<File> FileSP;
- // The SourceFileCache class separates the source manager from the cache of
- // source files, so the cache can be stored in the Debugger, but the source
- // managers can be per target.
+ /// The SourceFileCache class separates the source manager from the cache of
+ /// source files. There is one source manager per Target but both the Debugger
+ /// and the Process have their own source caches.
+ ///
+ /// The SourceFileCache just handles adding, storing, removing and looking up
+ /// source files. The caching policies are implemented in
+ /// SourceManager::GetFile.
class SourceFileCache {
public:
SourceFileCache() = default;
~SourceFileCache() = default;
void AddSourceFile(const FileSpec &file_spec, FileSP file_sp);
+ void RemoveSourceFile(const FileSP &file_sp);
+
FileSP FindSourceFile(const FileSpec &file_spec) const;
// Removes all elements from the cache.
@@ -117,15 +125,17 @@
typedef std::map<FileSpec, FileSP> FileCache;
FileCache m_file_cache;
+
+ mutable llvm::sys::RWMutex m_mutex;
};
- // Constructors and Destructors
- // A source manager can be made with a non-null target, in which case it can
- // use the path remappings to find
- // source files that are not in their build locations. With no target it
- // won't be able to do this.
+ /// A source manager can be made with a valid Target, in which case it can use
+ /// the path remappings to find source files that are not in their build
+ /// locations. Without a target it won't be able to do this.
+ /// @{
SourceManager(const lldb::DebuggerSP &debugger_sp);
SourceManager(const lldb::TargetSP &target_sp);
+ /// @}
~SourceManager();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
