Add logging and telemetry

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: https://phabricator.intern.facebook.com/D60673160

Author: royitaqi

lldb/include/lldb/Target/Statistics.h

@@ -184,6 +184,8 @@ class TargetStats {
   void SetFirstPrivateStopTime();
   void SetFirstPublicStopTime();
   void IncreaseSourceMapDeduceCount();
+  void IncreaseSourceRealpathAttemptCount();
+  void IncreaseSourceRealpathCompatibleCount();
 
   StatsDuration &GetCreateTime() { return m_create_time; }
   StatsSuccessFail &GetExpressionStats() { return m_expr_eval; }
@@ -198,6 +200,8 @@ class TargetStats {
   StatsSuccessFail m_frame_var{"frameVariable"};
   std::vector<intptr_t> m_module_identifiers;
   uint32_t m_source_map_deduce_count = 0;
+  uint32_t m_source_realpath_attempt_count = 0;
+  uint32_t m_source_realpath_compatible_count = 0;
   void CollectStats(Target &target);
 };
 

lldb/include/lldb/Utility/RealpathPrefixes.h

@@ -19,6 +19,7 @@
 namespace lldb_private {
 class FileSpec;
 class FileSpecList;
+class Target;
 } // namespace lldb_private
 
 namespace lldb_private {
@@ -35,6 +36,10 @@ class RealpathPrefixes {
   // filesystem will be used.
   void SetFileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
 
+  // Sets an optional Target instance to gather statistics.
+  void SetTarget(Target *target) { m_target = target; }
+  Target *GetTarget() const { return m_target; }
+
   std::optional<FileSpec> ResolveSymlinks(const FileSpec &file_spec) const;
 
 private:
@@ -48,6 +53,9 @@ class RealpathPrefixes {
 
   // The filesystem to use for realpath'ing.
   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
+
+  // The optional Target instance to gather statistics.
+  Target *m_target;
 };
 
 } // namespace lldb_private

lldb/source/Target/Statistics.cpp

@@ -192,6 +192,10 @@ TargetStats::ToJSON(Target &target,
   }
   target_metrics_json.try_emplace("sourceMapDeduceCount",
                                   m_source_map_deduce_count);
+  target_metrics_json.try_emplace("sourceRealpathAttemptCount",
+                                  m_source_realpath_attempt_count);
+  target_metrics_json.try_emplace("sourceRealpathCompatibleCount",
+                                  m_source_realpath_compatible_count);
   return target_metrics_json;
 }
 
@@ -220,6 +224,14 @@ void TargetStats::IncreaseSourceMapDeduceCount() {
   ++m_source_map_deduce_count;
 }
 
+void TargetStats::IncreaseSourceRealpathAttemptCount() {
+  ++m_source_realpath_attempt_count;
+}
+
+void TargetStats::IncreaseSourceRealpathCompatibleCount() {
+  ++m_source_realpath_compatible_count;
+}
+
 bool DebuggerStats::g_collecting_stats = false;
 
 llvm::json::Value DebuggerStats::ReportStatistics(

lldb/source/Target/Target.cpp

@@ -4359,7 +4359,9 @@ InlineStrategy TargetProperties::GetInlineStrategy() const {
 // this because we want the FileSpecList to normalize the file paths for us.
 RealpathPrefixes TargetProperties::GetSourceRealpathPrefixes() const {
   const uint32_t idx = ePropertySourceRealpathPrefixes;
-  return RealpathPrefixes(GetPropertyAtIndexAs<FileSpecList>(idx, {}));
+  auto prefixes = RealpathPrefixes(GetPropertyAtIndexAs<FileSpecList>(idx, {}));
+  prefixes.SetTarget(this->m_target);
+  return prefixes;
 }
 
 llvm::StringRef TargetProperties::GetArg0() const {

lldb/source/Utility/FileSpecList.cpp

@@ -7,7 +7,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/FileSpecList.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 #include "lldb/Utility/RealpathPrefixes.h"
 #include "lldb/Utility/Stream.h"
 
@@ -176,8 +180,17 @@ size_t SupportFileList::FindCompatibleIndex(
     if (realpath_prefixes && result == IsCompatibleResult::kOnlyFileMatch) {
       if (std::optional<FileSpec> resolved_curr_file =
               realpath_prefixes->ResolveSymlinks(curr_file)) {
-        if (IsCompatible(*resolved_curr_file, file_spec))
+        if (IsCompatible(*resolved_curr_file, file_spec)) {
+          // Stats and logging.
+          if (Target *target = realpath_prefixes->GetTarget())
+            target->GetStatistics().IncreaseSourceRealpathCompatibleCount();
+          Log *log = GetLog(LLDBLog::Source);
+          LLDB_LOGF(log,
+                    "Realpath'ed support file %s is compatible to input file",
+                    resolved_curr_file->GetPath().c_str());
+          // We found a match
           return idx;
+        }
       }
     }
   }

lldb/source/Utility/RealpathPrefixes.cpp

@@ -8,14 +8,17 @@
 
 #include "lldb/Utility/RealpathPrefixes.h"
 
-#include "lldb/Host/FileSystem.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Target/Target.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/FileSpecList.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
 
 using namespace lldb_private;
 
 RealpathPrefixes::RealpathPrefixes(const FileSpecList &file_spec_list)
-    : m_fs(llvm::vfs::getRealFileSystem()) {
+    : m_fs(llvm::vfs::getRealFileSystem()), m_target(nullptr) {
   m_prefixes.reserve(file_spec_list.GetSize());
   for (const FileSpec &file_spec : file_spec_list) {
     m_prefixes.emplace_back(file_spec.GetPath());
@@ -39,12 +42,18 @@ RealpathPrefixes::ResolveSymlinks(const FileSpec &file_spec) const {
   std::string file_spec_path = file_spec.GetPath();
   for (const std::string &prefix : m_prefixes) {
     if (is_prefix(file_spec_path, prefix, file_spec.IsCaseSensitive())) {
+      // Stats and logging.
+      if (m_target)
+        m_target->GetStatistics().IncreaseSourceRealpathAttemptCount();
+      Log *log = GetLog(LLDBLog::Source);
+      LLDB_LOGF(log, "Realpath'ing support file %s", file_spec_path.c_str());
+
       // One prefix matched. Try to realpath.
       llvm::SmallString<PATH_MAX> buff;
       std::error_code ec = m_fs->getRealPath(file_spec_path, buff);
       if (ec)
         return std::nullopt;
-      FileSpec realpath(buff);
+      FileSpec realpath(buff, file_spec.GetPathStyle());
 
       // Only return realpath if it is different from the original file_spec.
       if (realpath != file_spec)

lldb/unittests/Utility/CMakeLists.txt

@@ -51,6 +51,7 @@ add_lldb_unittest(UtilityTests
   XcodeSDKTest.cpp
 
   LINK_LIBS
+      lldbTarget
       lldbUtility
       lldbUtilityHelpers
       LLVMTestingSupport

lldb/unittests/Utility/MockSymlinkFileSystem.h

@@ -20,14 +20,16 @@ class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
 
   /// Treat \a symlink as a symlink to \a realpath. Treat all other files as
   /// non-symlinks.
-  MockSymlinkFileSystem(FileSpec &&symlink, FileSpec &&realpath)
-      : m_symlink(std::move(symlink)), m_realpath(std::move(realpath)) {}
+  MockSymlinkFileSystem(FileSpec &&symlink, FileSpec &&realpath,
+                        FileSpec::Style style = FileSpec::Style::native)
+      : m_symlink(std::move(symlink)), m_realpath(std::move(realpath)),
+        m_style(style) {}
 
   /// If \a Path matches the symlink given in the ctor, put the realpath given
   /// in the ctor into \a Output.
   std::error_code getRealPath(const llvm::Twine &Path,
                               llvm::SmallVectorImpl<char> &Output) override {
-    if (FileSpec(Path.str()) == m_symlink) {
+    if (FileSpec(Path.str(), m_style) == m_symlink) {
       std::string path = m_realpath.GetPath();
       Output.assign(path.begin(), path.end());
     } else {
@@ -58,6 +60,7 @@ class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
 private:
   FileSpec m_symlink;
   FileSpec m_realpath;
+  FileSpec::Style m_style;
 };
 
 } // namespace lldb_private

lldb/unittests/Utility/RealpathPrefixesTest.cpp

@@ -51,23 +51,24 @@ TEST(RealpathPrefixesTest, MatchingRelativePrefix) {
   EXPECT_EQ(ret, FileSpec("dir2/real.h"));
 }
 
-// Should resolve a symlink which match a prefix with a case-insensitive support
-// file
-TEST(RealpathPrefixesTest, CaseInsensitive) {
+// Should resolve in Windows and/or with a case-insensitive support file
+TEST(RealpathPrefixesTest, WindowsAndCaseInsensitive) {
   // Prepare FS
   llvm::IntrusiveRefCntPtr<MockSymlinkFileSystem> fs(new MockSymlinkFileSystem(
-      FileSpec("/dir1/link.h"), FileSpec("/dir2/real.h")));
+      FileSpec("f:\\dir1\\link.h", FileSpec::Style::windows),
+      FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows),
+      FileSpec::Style::windows));
 
   // Prepare RealpathPrefixes
   FileSpecList file_spec_list;
-  file_spec_list.EmplaceBack("/dir1");
+  file_spec_list.EmplaceBack(FileSpec("f:\\dir1", FileSpec::Style::windows));
   RealpathPrefixes prefixes(std::move(file_spec_list));
   prefixes.SetFileSystem(fs);
 
   // Test
   std::optional<FileSpec> ret = prefixes.ResolveSymlinks(
-      FileSpec("/DIR1/LINK.H", FileSpec::Style::posix));
-  EXPECT_EQ(ret, FileSpec("/dir2/real.h"));
+      FileSpec("F:\\DIR1\\LINK.H", FileSpec::Style::windows));
+  EXPECT_EQ(ret, FileSpec("f:\\dir2\\real.h", FileSpec::Style::windows));
 }
 
 // Should resolve a symlink when there is mixture of matching and mismatching