[Lldb-commits] [lldb] dabe877 - Cache the value for absolute path in FileSpec.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Fri Jul 22 14:05:00 PDT 2022
Author: Greg Clayton
Date: 2022-07-22T14:04:52-07:00
New Revision: dabe877248b85b34878e75d5510339325ee087d0
URL: https://github.com/llvm/llvm-project/commit/dabe877248b85b34878e75d5510339325ee087d0
DIFF: https://github.com/llvm/llvm-project/commit/dabe877248b85b34878e75d5510339325ee087d0.diff
LOG: Cache the value for absolute path in FileSpec.
Checking if a path is absolute can be expensive and currently the result is not cached in the FileSpec object. This patch adds caching and also code to clear the cache if the file is modified.
Differential Revision: https://reviews.llvm.org/D130396
Added:
Modified:
lldb/include/lldb/Utility/FileSpec.h
lldb/source/Utility/FileSpec.cpp
lldb/unittests/Utility/FileSpecTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/FileSpec.h b/lldb/include/lldb/Utility/FileSpec.h
index a0b01fb085ebe..8492c93fd58a1 100644
--- a/lldb/include/lldb/Utility/FileSpec.h
+++ b/lldb/include/lldb/Utility/FileSpec.h
@@ -416,10 +416,24 @@ class FileSpec {
// Convenience method for setting the file without changing the style.
void SetFile(llvm::StringRef path);
+ /// Called anytime m_directory or m_filename is changed to clear any cached
+ /// state in this object.
+ void PathWasModified() {
+ m_is_resolved = false;
+ m_absolute = Absolute::Calculate;
+ }
+
+ enum class Absolute : uint8_t {
+ Calculate,
+ Yes,
+ No
+ };
+
// Member variables
ConstString m_directory; ///< The uniqued directory path
ConstString m_filename; ///< The uniqued filename path
mutable bool m_is_resolved = false; ///< True if this path has been resolved.
+ mutable Absolute m_absolute = Absolute::Calculate; ///< Cache absoluteness.
Style m_style; ///< The syntax that this path uses (e.g. Windows / Posix)
};
diff --git a/lldb/source/Utility/FileSpec.cpp b/lldb/source/Utility/FileSpec.cpp
index 7646d33264226..48b922970d87e 100644
--- a/lldb/source/Utility/FileSpec.cpp
+++ b/lldb/source/Utility/FileSpec.cpp
@@ -170,9 +170,7 @@ void FileSpec::SetFile(llvm::StringRef pathname) { SetFile(pathname, m_style); }
// up into a directory and filename and stored as uniqued string values for
// quick comparison and efficient memory usage.
void FileSpec::SetFile(llvm::StringRef pathname, Style style) {
- m_filename.Clear();
- m_directory.Clear();
- m_is_resolved = false;
+ Clear();
m_style = (style == Style::native) ? GetNativeStyle() : style;
if (pathname.empty())
@@ -259,6 +257,7 @@ Stream &lldb_private::operator<<(Stream &s, const FileSpec &f) {
void FileSpec::Clear() {
m_directory.Clear();
m_filename.Clear();
+ PathWasModified();
}
// Compare two FileSpec objects. If "full" is true, then both the directory and
@@ -332,26 +331,32 @@ FileSpec::Style FileSpec::GetPathStyle() const { return m_style; }
void FileSpec::SetDirectory(ConstString directory) {
m_directory = directory;
+ PathWasModified();
}
void FileSpec::SetDirectory(llvm::StringRef directory) {
m_directory = ConstString(directory);
+ PathWasModified();
}
void FileSpec::SetFilename(ConstString filename) {
m_filename = filename;
+ PathWasModified();
}
void FileSpec::SetFilename(llvm::StringRef filename) {
m_filename = ConstString(filename);
+ PathWasModified();
}
void FileSpec::ClearFilename() {
m_filename.Clear();
+ PathWasModified();
}
void FileSpec::ClearDirectory() {
m_directory.Clear();
+ PathWasModified();
}
// Extract the directory and path into a fixed buffer. This is needed as the
@@ -488,18 +493,22 @@ bool FileSpec::IsRelative() const {
}
bool FileSpec::IsAbsolute() const {
-llvm::SmallString<64> current_path;
- GetPath(current_path, false);
+ // Check if we have cached if this path is absolute to avoid recalculating.
+ if (m_absolute != Absolute::Calculate)
+ return m_absolute == Absolute::Yes;
- // Early return if the path is empty.
- if (current_path.empty())
- return false;
+ m_absolute = Absolute::No;
- // We consider paths starting with ~ to be absolute.
- if (current_path[0] == '~')
- return true;
+ llvm::SmallString<64> path;
+ GetPath(path, false);
+
+ if (!path.empty()) {
+ // We consider paths starting with ~ to be absolute.
+ if (path[0] == '~' || llvm::sys::path::is_absolute(path, m_style))
+ m_absolute = Absolute::Yes;
+ }
- return llvm::sys::path::is_absolute(current_path, m_style);
+ return m_absolute == Absolute::Yes;
}
void FileSpec::MakeAbsolute(const FileSpec &dir) {
diff --git a/lldb/unittests/Utility/FileSpecTest.cpp b/lldb/unittests/Utility/FileSpecTest.cpp
index 9260262d8b674..0249dd5a98a4d 100644
--- a/lldb/unittests/Utility/FileSpecTest.cpp
+++ b/lldb/unittests/Utility/FileSpecTest.cpp
@@ -450,3 +450,29 @@ TEST(FileSpecTest, OperatorBool) {
EXPECT_FALSE(FileSpec(""));
EXPECT_TRUE(FileSpec("/foo/bar"));
}
+
+
+TEST(FileSpecTest, TestAbsoluteCaching) {
+ // Test that if we modify a path that we recalculate if a path is relative
+ // or absolute correctly. The test below calls set accessors and functions
+ // that change the path and verifies that the FileSpec::IsAbsolute() returns
+ // the correct value.
+ FileSpec file = PosixSpec("/tmp/a");
+ EXPECT_TRUE(file.IsAbsolute());
+ file.ClearDirectory();
+ EXPECT_FALSE(file.IsAbsolute());
+ file.SetDirectory("/tmp");
+ EXPECT_TRUE(file.IsAbsolute());
+ file.SetDirectory(".");
+ EXPECT_FALSE(file.IsAbsolute());
+ file.SetPath("/log.txt");
+ EXPECT_TRUE(file.IsAbsolute());
+ file.RemoveLastPathComponent();
+ EXPECT_TRUE(file.IsAbsolute());
+ file.ClearFilename();
+ EXPECT_FALSE(file.IsAbsolute());
+ file.AppendPathComponent("foo.txt");
+ EXPECT_FALSE(file.IsAbsolute());
+ file.PrependPathComponent("/tmp");
+ EXPECT_TRUE(file.IsAbsolute());
+}
More information about the lldb-commits
mailing list