We recently saw an uptick in internal reports complaining that LLDB is
slow when sources on network file systems are inaccessible. I looked at
the SourceManger and its cache and I think there’s some room for
improvement in terms of reducing file system accesses:
1. We always resolve the path.
2. We always check the timestamp.
3. We always recheck the file system for negative cache hits.
D153726 fixes (1) but (2) and (3) are necessary because of the cache’s
current design. Source files are cached at the debugger level which
means that the source file cache can span multiple targets and
processes. It wouldn't be correct to not reload a modified or new file
from disk.
We can however significantly reduce the number of file system accesses
by using a two level cache design: one cache at the debugger level and
one at the process level:
- The cache at the debugger level works the way it does today. There is
no negative cache: if we can't find the file on disk, we'll try again
next time the cache is queried. If a cached file's timestamp changes
or if its path remapping changes, the cached file is evicted and we
reload it from disk.
- The cache at the process level is design to avoid accessing the file
system. It doesn't check the file's modification time. It caches
negative results, so if a file didn't exist, it doesn't try to reread
it from disk. Checking if the path remapping changed is cheap
(doesn't involve checking the file system) and is the only way for a
file to get evicted from the process cache.
The result of this patch is that LLDB will not show you new content if a
file is modified or created while a process is running. I would argue
that this is what most people would expect, but it is a change from how
LLDB behaves today.
For an average stop, we query the source cache 4 times. With the current
implementation, that's 4 stats to get the modification time, If the file
doesn't exist on disk, that's an additional 4 stats. Before D153726, if
the path starts with a ~ there are another additional 4 calls to
realpath. When debugging sources on a slow (network) file system, this
quickly adds up.
In addition to the two level caching, this patch also adds a source
logging channel and synchronization to the source file cache. The
logging was helpful during development and hopefully will help us triage
issues in the future. The synchronization isn't a new requirement: as
the cache is shared across targets, there is no guarantees that it can't
be accessed concurrently. The patch also fixes a bug where we would only
set the source remapping ID if the un-remapped file didn't exist, which
led to the file getting evicted from the cache on every access.
rdar://
110787562
Differential revision: https://reviews.llvm.org/D153834
#include "lldb/lldb-forward.h"
#include "llvm/Support/Chrono.h"
+#include "llvm/Support/RWMutex.h"
#include <cstddef>
#include <cstdint>
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,
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.
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();
#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"
virtual void ForceScriptedState(lldb::StateType state) {}
+ SourceManager::SourceFileCache &GetSourceFileCache() {
+ return m_source_file_cache;
+ }
+
protected:
friend class Trace;
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;
Unwind = Log::ChannelFlag<29>,
Watchpoints = Log::ChannelFlag<30>,
OnDemand = Log::ChannelFlag<31>,
+ Source = Log::ChannelFlag<32>,
LLVM_MARK_AS_BITMASK_ENUM(OnDemand),
};
#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"
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();
}
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();
}
#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"
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;
}
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),
// 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)
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,
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);
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);
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 {
{{"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,
"""
import os
+import io
import stat
import lldb
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."""
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,
# 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"],
+ )
+
// 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.
// 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.
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);
projects / platform / upstream / llvm.git / commitdiff