Revert "Allow signposts to take advantage of deferred string substitution"

This reverts commits f9aba9a5afe09788eceb9879aa5c3ad345e0f1e9 and
035217ff515b8ecdc871e39fa840f3cba1b9cec7.

As explained in the original commit message, this didn't have the
intended effect of improving the common LLDB use case, but still
provided a marginal improvement for the places where LLDB creates a
scoped time with a string literal.

The reason for the revert is that this change pulls in the os/signpost.h
header in Signposts.h. The former transitively includes loader.h, which
contains a series of macro defines that conflict with MachO.h. There are
ways to work around that, but Adrian and I concluded that  none of them
are worth the trade-off in complicating Signposts.h even further.

diff --git a/lldb/include/lldb/Utility/Timer.h b/lldb/include/lldb/Utility/Timer.h
index c70c1804942665a25e2ee0f094394b126d3424ae..201378bbeb2cc54041f5b143a5a8ce4de985f3d6 100644 (file)
--- a/lldb/include/lldb/Utility/Timer.h
+++ b/lldb/include/lldb/Utility/Timer.h
@@ -9,16 +9,11 @@
 #ifndef LLDB_UTILITY_TIMER_H
 #define LLDB_UTILITY_TIMER_H
 
-#include "llvm/ADT/ScopeExit.h"
+#include "lldb/lldb-defines.h"
 #include "llvm/Support/Chrono.h"
-#include "llvm/Support/Signposts.h"
 #include <atomic>
 #include <cstdint>
 
-namespace llvm {
-  class SignpostEmitter;
-}
-
 namespace lldb_private {
 class Stream;
 
@@ -81,28 +76,15 @@ private:
   const Timer &operator=(const Timer &) = delete;
 };
 
-llvm::SignpostEmitter &GetSignposts();
-
 } // namespace lldb_private
 
 // Use a format string because LLVM_PRETTY_FUNCTION might not be a string
 // literal.
 #define LLDB_SCOPED_TIMER()                                                    \
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);           \
-  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION);       \
-  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),              \
-                                  &_scoped_timer, "%s", LLVM_PRETTY_FUNCTION); \
-  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {           \
-    ::lldb_private::GetSignposts().endInterval(&_scoped_timer);                \
-  })
-
-#define LLDB_SCOPED_TIMERF(FMT, ...)                                           \
+  ::lldb_private::Timer _scoped_timer(_cat, "%s", LLVM_PRETTY_FUNCTION)
+#define LLDB_SCOPED_TIMERF(...)                                                \
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);           \
-  ::lldb_private::Timer _scoped_timer(_cat, FMT, __VA_ARGS__);                 \
-  SIGNPOST_EMITTER_START_INTERVAL(::lldb_private::GetSignposts(),              \
-                                  &_scoped_timer, FMT, __VA_ARGS__);           \
-  auto _scoped_signpost = llvm::make_scope_exit([&_scoped_timer]() {           \
-    ::lldb_private::GetSignposts().endInterval(&_scoped_timer);                \
-  })
+  ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)
 
 #endif // LLDB_UTILITY_TIMER_H

diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index ff8e60df393f13c31efb235f1ce9f37df0336c1a..b3892b016c2e1db80039f27f521e0c3fb2553bdd 100644 (file)
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -21,7 +21,6 @@
 #include "lldb/Core/Section.h"
 #include "lldb/Core/StreamFile.h"
 #include "lldb/Host/Host.h"
-#include "lldb/Host/SafeMachO.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"
 #include "lldb/Symbol/LocateSymbolFile.h"
 #include "lldb/Symbol/ObjectFile.h"
@@ -44,6 +43,8 @@
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/UUID.h"
 
+#include "lldb/Host/SafeMachO.h"
+
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -65,48 +66,65 @@
 #include <bitset>
 #include <memory>
 
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
 // Unfortunately the signpost header pulls in the system MachO header, too.
+#ifdef CPU_TYPE_ARM
 #undef CPU_TYPE_ARM
+#endif
+#ifdef CPU_TYPE_ARM64
 #undef CPU_TYPE_ARM64
+#endif
+#ifdef CPU_TYPE_ARM64_32
 #undef CPU_TYPE_ARM64_32
+#endif
+#ifdef CPU_TYPE_I386
 #undef CPU_TYPE_I386
+#endif
+#ifdef CPU_TYPE_X86_64
 #undef CPU_TYPE_X86_64
-#undef MH_BINDATLOAD
-#undef MH_BUNDLE
-#undef MH_CIGAM
-#undef MH_CIGAM_64
-#undef MH_CORE
-#undef MH_DSYM
-#undef MH_DYLDLINK
-#undef MH_DYLIB
-#undef MH_DYLIB_STUB
-#undef MH_DYLINKER
+#endif
+#ifdef MH_DYLINKER
 #undef MH_DYLINKER
-#undef MH_EXECUTE
-#undef MH_FVMLIB
-#undef MH_INCRLINK
-#undef MH_KEXT_BUNDLE
-#undef MH_MAGIC
-#undef MH_MAGIC_64
-#undef MH_NOUNDEFS
-#undef MH_OBJECT
+#endif
+#ifdef MH_OBJECT
 #undef MH_OBJECT
-#undef MH_PRELOAD
-
-#undef LC_BUILD_VERSION
+#endif
+#ifdef LC_VERSION_MIN_MACOSX
 #undef LC_VERSION_MIN_MACOSX
+#endif
+#ifdef LC_VERSION_MIN_IPHONEOS
 #undef LC_VERSION_MIN_IPHONEOS
+#endif
+#ifdef LC_VERSION_MIN_TVOS
 #undef LC_VERSION_MIN_TVOS
+#endif
+#ifdef LC_VERSION_MIN_WATCHOS
 #undef LC_VERSION_MIN_WATCHOS
-
+#endif
+#ifdef LC_BUILD_VERSION
+#undef LC_BUILD_VERSION
+#endif
+#ifdef PLATFORM_MACOS
 #undef PLATFORM_MACOS
+#endif
+#ifdef PLATFORM_MACCATALYST
 #undef PLATFORM_MACCATALYST
+#endif
+#ifdef PLATFORM_IOS
 #undef PLATFORM_IOS
+#endif
+#ifdef PLATFORM_IOSSIMULATOR
 #undef PLATFORM_IOSSIMULATOR
+#endif
+#ifdef PLATFORM_TVOS
 #undef PLATFORM_TVOS
+#endif
+#ifdef PLATFORM_TVOSSIMULATOR
 #undef PLATFORM_TVOSSIMULATOR
+#endif
+#ifdef PLATFORM_WATCHOS
 #undef PLATFORM_WATCHOS
+#endif
+#ifdef PLATFORM_WATCHOSSIMULATOR
 #undef PLATFORM_WATCHOSSIMULATOR
 #endif
 

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 35ce2bc5c39ea0d4655f4a959cacef1d6e589acb..64532139855ab3602c25271a881069aac83c6bdc 100644 (file)
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Utility/RangeMap.h"
 #include "lldb/Utility/RegularExpression.h"
+#include "lldb/Utility/Timer.h"
 
 //#define DEBUG_OSO_DMAP // DO NOT CHECKIN WITH THIS NOT COMMENTED OUT
 #if defined(DEBUG_OSO_DMAP)
@@ -33,9 +34,6 @@
 #include "LogChannelDWARF.h"
 #include "SymbolFileDWARF.h"
 
-// Work around the fact that Timer.h pulls in the system Mach-O headers.
-#include "lldb/Utility/Timer.h"
-
 #include <memory>
 
 using namespace lldb;

diff --git a/lldb/source/Utility/Timer.cpp b/lldb/source/Utility/Timer.cpp
index b59ce3b9f55632e7e17249d7d066d91c6d8f8e8e..2f3afe4c870371e1688166fd0628b4355c213e1b 100644 (file)
--- a/lldb/source/Utility/Timer.cpp
+++ b/lldb/source/Utility/Timer.cpp
@@ -33,8 +33,6 @@ static std::atomic<Timer::Category *> g_categories;
 /// Allows llvm::Timer to emit signposts when supported.
 static llvm::ManagedStatic<llvm::SignpostEmitter> Signposts;
 
-llvm::SignpostEmitter &lldb_private::GetSignposts() { return *Signposts; }
-
 std::atomic<bool> Timer::g_quiet(true);
 std::atomic<unsigned> Timer::g_display_depth(0);
 static std::mutex &GetFileMutex() {
@@ -61,6 +59,7 @@ void Timer::SetQuiet(bool value) { g_quiet = value; }
 
 Timer::Timer(Timer::Category &category, const char *format, ...)
     : m_category(category), m_total_start(std::chrono::steady_clock::now()) {
+  Signposts->startInterval(this, m_category.GetName());
   TimerStack &stack = GetTimerStackForCurrentThread();
 
   stack.push_back(this);
@@ -87,6 +86,8 @@ Timer::~Timer() {
   auto total_dur = stop_time - m_total_start;
   auto timer_dur = total_dur - m_child_duration;
 
+  Signposts->endInterval(this, m_category.GetName());
+
   TimerStack &stack = GetTimerStackForCurrentThread();
   if (g_quiet && stack.size() <= g_display_depth) {
     std::lock_guard<std::mutex> lock(GetFileMutex());

diff --git a/llvm/include/llvm/Config/config.h.cmake b/llvm/include/llvm/Config/config.h.cmake
index d7cd44b5db36a8cf3c352206c5da7c578e3a67e5..37a0d234844d190eefda4a02538907e52bb60f99 100644 (file)
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -353,6 +353,9 @@
 /* Define to the default GlobalISel coverage file prefix */
 #cmakedefine LLVM_GISEL_COV_PREFIX "${LLVM_GISEL_COV_PREFIX}"
 
+/* Whether Timers signpost passes in Xcode Instruments */
+#cmakedefine01 LLVM_SUPPORT_XCODE_SIGNPOSTS
+
 #cmakedefine HAVE_PROC_PID_RUSAGE 1
 
 #endif

diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake b/llvm/include/llvm/Config/llvm-config.h.cmake
index 82a979411209772305c7932461cfbd0b2a3b8905..169f59695e5ed1be1b6508cdba387663ed40b142 100644 (file)
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -100,8 +100,4 @@
 /* Define if the xar_open() function is supported on this platform. */
 #cmakedefine LLVM_HAVE_LIBXAR ${LLVM_HAVE_LIBXAR}
 
-/* Whether Timers signpost passes in Xcode Instruments */
-#cmakedefine01 LLVM_SUPPORT_XCODE_SIGNPOSTS
-
-
 #endif

diff --git a/llvm/include/llvm/Support/Signposts.h b/llvm/include/llvm/Support/Signposts.h
index e352bbd4e25cb4e4354fffbebe74d9875da24c3e..dabbba6f89d1a3b86a122f1edea0817944b7a55c 100644 (file)
--- a/llvm/include/llvm/Support/Signposts.h
+++ b/llvm/include/llvm/Support/Signposts.h
@@ -17,17 +17,8 @@
 #define LLVM_SUPPORT_SIGNPOSTS_H
 
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Config/llvm-config.h"
 #include <memory>
 
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
-#include <Availability.h>
-#include <os/signpost.h>
-#endif
-
-#define SIGNPOSTS_AVAILABLE()                                                  \
-  __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
-
 namespace llvm {
 class SignpostEmitterImpl;
 
@@ -44,33 +35,8 @@ public:
 
   /// Begin a signposted interval for a given object.
   void startInterval(const void *O, StringRef Name);
-
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
-  os_log_t &getLogger() const;
-  os_signpost_id_t getSignpostForObject(const void *O);
-#endif
-
-  /// A macro to take advantage of the special format string handling
-  /// in the os_signpost API. The format string substitution is
-  /// deferred to the log consumer and done outside of the
-  /// application.
-#if LLVM_SUPPORT_XCODE_SIGNPOSTS
-#define SIGNPOST_EMITTER_START_INTERVAL(SIGNPOST_EMITTER, O, ...)              \
-  do {                                                                         \
-    if ((SIGNPOST_EMITTER).isEnabled())                                        \
-      if (SIGNPOSTS_AVAILABLE())                                               \
-        os_signpost_interval_begin((SIGNPOST_EMITTER).getLogger(),             \
-                                   (SIGNPOST_EMITTER).getSignpostForObject(O), \
-                                   "LLVM Timers", __VA_ARGS__);                \
-  } while (0)
-#else
-#define SIGNPOST_EMITTER_START_INTERVAL(SIGNPOST_EMITTER, O, ...)              \
-  do {                                                                         \
-  } while (0)
-#endif
-
   /// End a signposted interval for a given object.
-  void endInterval(const void *O);
+  void endInterval(const void *O, StringRef Name);
 };
 
 } // end namespace llvm

diff --git a/llvm/lib/Support/Signposts.cpp b/llvm/lib/Support/Signposts.cpp
index 09df0220fb212675b0fe8e9b894aafe9c303f4ec..58fafb26cdf3a27ea1c9b9e4ec1755fe9b3b9996 100644 (file)
--- a/llvm/lib/Support/Signposts.cpp
+++ b/llvm/lib/Support/Signposts.cpp
@@ -9,14 +9,19 @@
 #include "llvm/Support/Signposts.h"
 #include "llvm/Support/Timer.h"
 
+#include "llvm/Config/config.h"
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/Mutex.h"
-#endif
+#include <Availability.h>
+#include <os/signpost.h>
+#endif // if LLVM_SUPPORT_XCODE_SIGNPOSTS
 
 using namespace llvm;
 
 #if LLVM_SUPPORT_XCODE_SIGNPOSTS
+#define SIGNPOSTS_AVAILABLE()                                                  \
+  __builtin_available(macos 10.14, iOS 12, tvOS 12, watchOS 5, *)
 namespace {
 os_log_t *LogCreator() {
   os_log_t *X = new os_log_t;
@@ -34,13 +39,13 @@ struct LogDeleter {
 namespace llvm {
 class SignpostEmitterImpl {
   using LogPtrTy = std::unique_ptr<os_log_t, LogDeleter>;
+  using LogTy = LogPtrTy::element_type;
 
   LogPtrTy SignpostLog;
   DenseMap<const void *, os_signpost_id_t> Signposts;
   sys::SmartMutex<true> Mutex;
 
-public:
-  os_log_t &getLogger() const { return *SignpostLog; }
+  LogTy &getLogger() const { return *SignpostLog; }
   os_signpost_id_t getSignpostForObject(const void *O) {
     sys::SmartScopedLock<true> Lock(Mutex);
     const auto &I = Signposts.find(O);
@@ -54,6 +59,7 @@ public:
     return Inserted.first->second;
   }
 
+public:
   SignpostEmitterImpl() : SignpostLog(LogCreator()) {}
 
   bool isEnabled() const {
@@ -72,7 +78,7 @@ public:
     }
   }
 
-  void endInterval(const void *O) {
+  void endInterval(const void *O, llvm::StringRef Name) {
     if (isEnabled()) {
       if (SIGNPOSTS_AVAILABLE()) {
         // Both strings used here are required to be constant literal strings.
@@ -118,17 +124,10 @@ void SignpostEmitter::startInterval(const void *O, StringRef Name) {
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }
 
-#if HAVE_ANY_SIGNPOST_IMPL
-os_log_t &SignpostEmitter::getLogger() const { return Impl->getLogger(); }
-os_signpost_id_t SignpostEmitter::getSignpostForObject(const void *O) {
-  return Impl->getSignpostForObject(O);
-}
-#endif
-
-void SignpostEmitter::endInterval(const void *O) {
+void SignpostEmitter::endInterval(const void *O, StringRef Name) {
 #if HAVE_ANY_SIGNPOST_IMPL
   if (Impl == nullptr)
     return;
-  Impl->endInterval(O);
+  Impl->endInterval(O, Name);
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }

diff --git a/llvm/lib/Support/Timer.cpp b/llvm/lib/Support/Timer.cpp
index f025ecd3d45c41992fb2453c23037672c7b0c9fd..d69199133f6a67695d211c65113c611c244f1b14 100644 (file)
--- a/llvm/lib/Support/Timer.cpp
+++ b/llvm/lib/Support/Timer.cpp
@@ -199,7 +199,7 @@ void Timer::stopTimer() {
   Running = false;
   Time += TimeRecord::getCurrentTime(false);
   Time -= StartTime;
-  Signposts->endInterval(this);
+  Signposts->endInterval(this, getName());
 }
 
 void Timer::clear() {