[Lldb-commits] [PATCH] D153834: [lldb] Add two-level caching in the source manager

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, clayborg, jingham, jasonmolenda.
Herald added a project: All.
JDevlieghere requested review of this revision.

We recently saw an uptick in internal reports complaining that LLDB is slow when source on NFS 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 <https://reviews.llvm.org/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) so this 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 cache four times. With the current implementation, that's 4 stats to get the modification time, If the file doesn't exist on disk, that's another 4 stats. Before D153726 <https://reviews.llvm.org/D153726>, if the path starts with a `~` there are another additional 4 calls to realpath. When debugging sources on a slow (network) filesystem, 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.


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153834.534837.patch
Type: text/x-patch
Size: 22190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230627/d1c8b400/attachment-0001.bin>