Allow signposts to take advantage of deferred string substitution

[aprantl]

Summary

This is more an RFC at this point. I'm curious about feedback!

One nice feature of the os_signpost API is that format string substitutions happen in the consumer, not the logging application. LLVM's current Signpost class doesn't take advantage of this though and instead always uses a static "Begin/End %s" format string.

This patch is proof of concept for how to use variadic macros to allow the API to be used as intended. Unfortunately, the primary use-case I had in mind (the LLDB_SCOPED_TIMER() macro) does not get much better from this, because __PRETTY_FUNCTION__ is *not* a macro, but a static string, so signposts created by LLDB_SCOPED_TIMER() still use a static "%s" format string. At least LLDB_SCOPED_TIMERF() works as intended.

[JDevlieghere]

I think we should just remove the functionality form the timer class again. I only added it there because of the macro.

[aprantl]

I'm not sure I fully understand the suggestion:

I think we should just remove the functionality form the timer class again. I only added it there because of the macro.

... and replace its uses with what?

[JDevlieghere]

In D103575#2804151, @aprantl wrote:

I'm not sure I fully understand the suggestion:

I think we should just remove the functionality form the timer class again. I only added it there because of the macro.

... and replace its uses with what?

I'm suggesting we no longer emit a signpost from the lldb_private::Timer class and instead exclusively emit a signpost from the LLDB_SCOPED_TIMER macro. We barely have any places where someone creates a timer directly and this would help discourage that further. The only reason it's currently in the timer class it because it seemed cleaner than doing it in the macro directly. Since we now have to emit the signpost from the LLDB_SCOPED_TIMER macro anyway, there's no reason to keep the functionality in the Timer class.

[aprantl]

Address review feedback from @JDevlieghere.

[aprantl]

Let me know if this goes into the right direction.

[JDevlieghere]

Looks good to me!

[fhahn]

It looks like this is causing build failures on certain macOS / SDK combinations, e.g. http://green.lab.llvm.org/green/job/lldb-cmake-standalone/3288/consoleFull#-195476041949ba4694-19c4-4d7e-bec5-911270d8a58c

I reverted the change for now in b4583a5ad73b

/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/host-compiler/bin/clang++  -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Isource/Plugins/ObjectFile/Mach-O -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O -Isource -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include -Iinclude -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/include -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/clang/include -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/clang-build/tools/clang/include -I/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/include/python3.7m -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/tools/clang/include -I../clang/include -I/usr/local/include -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/libxml2 -I/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/. -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk    -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT source/Plugins/ObjectFile/Mach-O/CMakeFiles/lldbPluginObjectFileMachO.dir/ObjectFileMachO.cpp.o -MF source/Plugins/ObjectFile/Mach-O/CMakeFiles/lldbPluginObjectFileMachO.dir/ObjectFileMachO.cpp.o.d -o source/Plugins/ObjectFile/Mach-O/CMakeFiles/lldbPluginObjectFileMachO.dir/ObjectFileMachO.cpp.o -c /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:45:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include/lldb/Host/SafeMachO.h:159:

/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include/llvm/BinaryFormat/MachO.h:30:3: error: expected identifier
  MH_MAGIC = 0xFEEDFACEu,
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/mach-o/loader.h:65:18: note: expanded from macro 'MH_MAGIC'
#define MH_MAGIC        0xfeedface      /* the mach magic number */
                        ^
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:45:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include/lldb/Host/SafeMachO.h:159:
/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include/llvm/BinaryFormat/MachO.h:31:3: error: expected identifier
  MH_CIGAM = 0xCEFAEDFEu,
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/mach-o/loader.h:66:18: note: expanded from macro 'MH_CIGAM'
#define MH_CIGAM        0xcefaedfe      /* NXSwapInt(MH_MAGIC) */
                        ^
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:45:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include/lldb/Host/SafeMachO.h:159:
/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include/llvm/BinaryFormat/MachO.h:32:3: error: expected identifier
  MH_MAGIC_64 = 0xFEEDFACFu,
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/mach-o/loader.h:84:21: note: expanded from macro 'MH_MAGIC_64'
#define MH_MAGIC_64 0xfeedfacf /* the 64-bit mach magic number */
                    ^
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:45:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include/lldb/Host/SafeMachO.h:159:
/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include/llvm/BinaryFormat/MachO.h:33:3: error: expected identifier
  MH_CIGAM_64 = 0xCFFAEDFEu,
  ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/mach-o/loader.h:85:21: note: expanded from macro 'MH_CIGAM_64'
#define MH_CIGAM_64 0xcffaedfe /* NXSwapInt(MH_MAGIC_64) */
                    ^
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:45:
In file included from /Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/lldb/include/lldb/Host/SafeMachO.h:159:
/Users/buildslave/jenkins/workspace/lldb-cmake-standalone/llvm-project/llvm/include/llvm/BinaryFormat/MachO.h:43:3: error: expected identifier
  MH_OBJECT = 0x1u,
  ^

[stella.stamenova]

The Windows buildbot does not like signposts:

https://lab.llvm.org/buildbot/#/builders/83/builds/7271

[aprantl]

In D103575#2818340, @stella.stamenova wrote:

The Windows buildbot does not like signposts:

https://lab.llvm.org/buildbot/#/builders/83/builds/7271

C:\PROGRA~2\MICROS~3\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lldb\source\Utility -IC:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\source\Utility -Itools\lldb\source -IC:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include -Itools\lldb\include -Iinclude -IC:\buildbot\lldb-x64-windows-ninja\llvm-project\llvm\include -I"C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64\include" -IC:\buildbot\lldb-x64-windows-ninja\llvm-project\llvm\..\clang\include -Itools\lldb\..\clang\include -IC:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\source\. /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Zc:strictStrings /Oi /Zc:rvalueCast /bigobj /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2 /DNDEBUG   -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530  /EHs-c- /GR- -std:c++14 /showIncludes /Fotools\lldb\source\Utility\CMakeFiles\lldbUtility.dir\Timer.cpp.obj /Fdtools\lldb\source\Utility\CMakeFiles\lldbUtility.dir\lldbUtility.pdb /FS -c C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\source\Utility\Timer.cpp
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include\lldb/Utility/Timer.h(49): error C3646: '__attribute__': unknown override specifier
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include\lldb/Utility/Timer.h(49): error C2059: syntax error: '('
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include\lldb/Utility/Timer.h(50): error C2059: syntax error: '('
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include\lldb/Utility/Timer.h(53): error C2143: syntax error: missing ')' before ';'
C:\buildbot\lldb-x64-windows-ninja\llvm-project\lldb\include\lldb/Utility/Timer.h(53): error C2059: syntax error: ')'

Looks like it's the attribute that it doesn't know about:

/// Default constructor.
Timer(Category &category, const char *format, ...)
    __attribute__((format(printf, 3, 4)));

[aprantl]

which is weird because I didn't actually change that.

[aprantl]

I tried to work around it.

lldb/include/lldb/Utility/Timer.h

 //===-- Timer.h -------------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_UTILITY_TIMER_H
 #define LLDB_UTILITY_TIMER_H
 
 #include "lldb/lldb-defines.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Signposts.h"
 #include <atomic>
 #include <cstdint>
 
+namespace llvm {
+  class SignpostEmitter;
+}
+
 namespace lldb_private {
 class Stream;
 
 /// \class Timer Timer.h "lldb/Utility/Timer.h"
 /// A timer class that simplifies common timing metrics.
 
 class Timer {
 public:
protected:
   static std::atomic<bool> g_quiet;
   static std::atomic<unsigned> g_display_depth;
 
 private:
   Timer(const Timer &) = delete;
   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)
-#define LLDB_SCOPED_TIMERF(...)                                                \
+  ::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, ...)                                           \
   static ::lldb_private::Timer::Category _cat(LLVM_PRETTY_FUNCTION);           \
-  ::lldb_private::Timer _scoped_timer(_cat, __VA_ARGS__)
+  ::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);                \
+  })
 
 #endif // LLDB_UTILITY_TIMER_H

lldb/source/Utility/Timer.cpp

 //===-- Timer.cpp ---------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "lldb/Utility/Timer.h"
 #include "lldb/Utility/Stream.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Signposts.h"
 
 #include <algorithm>
 #include <map>
 #include <mutex>
 #include <utility>

 namespace {
 typedef std::vector<Timer *> TimerStack;
 static std::atomic<Timer::Category *> g_categories;
 } // end of anonymous namespace
 
 /// 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() {
   static std::mutex *g_file_mutex_ptr = new std::mutex();
   return *g_file_mutex_ptr;
 }
 
 static TimerStack &GetTimerStackForCurrentThread() {
do {
     m_next = expected;
   } while (!g_categories.compare_exchange_weak(expected, this));
 }
 
 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);
   if (g_quiet && stack.size() <= g_display_depth) {
     std::lock_guard<std::mutex> lock(GetFileMutex());
 
     // Indent
     ::fprintf(stdout, "%*s", int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "");

 
 Timer::~Timer() {
   using namespace std::chrono;
 
   auto stop_time = steady_clock::now();
   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());
     ::fprintf(stdout, "%*s%.9f sec (%.9f sec)\n",
               int(stack.size() - 1) * TIMER_INDENT_AMOUNT, "",
               duration<double>(total_dur).count(),
               duration<double>(timer_dur).count());
   }

llvm/include/llvm/Support/Signposts.h

 /// of Interest track on the instruments timeline.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_SIGNPOSTS_H
 #define LLVM_SUPPORT_SIGNPOSTS_H
 
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Config/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;
 
 /// Manages the emission of signposts into the recording method supported by
 /// the OS.
 class SignpostEmitter {
   std::unique_ptr<SignpostEmitterImpl> Impl;
 
 public:
   SignpostEmitter();
   ~SignpostEmitter();
 
   bool isEnabled() const;
 
   /// 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.
+#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)
+
   /// End a signposted interval for a given object.
-  void endInterval(const void *O, StringRef Name);
+  void endInterval(const void *O);
 };
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_SIGNPOSTS_H

llvm/lib/Support/Signposts.cpp

 //===-- Signposts.cpp - Interval debug annotations ------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #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"
-#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;
   *X = os_log_create("org.llvm.signposts", OS_LOG_CATEGORY_POINTS_OF_INTEREST);
   return X;
 }
 struct LogDeleter {
   void operator()(os_log_t *X) const {
     os_release(*X);
     delete X;
   }
 };
 } // end anonymous namespace
 
 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;
 
-  LogTy &getLogger() const { return *SignpostLog; }
+public:
+  os_log_t &getLogger() const { return *SignpostLog; }
   os_signpost_id_t getSignpostForObject(const void *O) {
     sys::SmartScopedLock<true> Lock(Mutex);
     const auto &I = Signposts.find(O);
     if (I != Signposts.end())
       return I->second;
     os_signpost_id_t ID = {};
     if (SIGNPOSTS_AVAILABLE()) {
       ID = os_signpost_id_make_with_pointer(getLogger(), O);
     }
     const auto &Inserted = Signposts.insert(std::make_pair(O, ID));
     return Inserted.first->second;
   }
 
-public:
   SignpostEmitterImpl() : SignpostLog(LogCreator()) {}
 
   bool isEnabled() const {
     if (SIGNPOSTS_AVAILABLE())
       return os_signpost_enabled(*SignpostLog);
     return false;
   }
 
   void startInterval(const void *O, llvm::StringRef Name) {
     if (isEnabled()) {
       if (SIGNPOSTS_AVAILABLE()) {
         // Both strings used here are required to be constant literal strings.
         os_signpost_interval_begin(getLogger(), getSignpostForObject(O),
                                    "LLVM Timers", "%s", Name.data());
       }
     }
   }
 
-  void endInterval(const void *O, llvm::StringRef Name) {
+  void endInterval(const void *O) {
     if (isEnabled()) {
       if (SIGNPOSTS_AVAILABLE()) {
         // Both strings used here are required to be constant literal strings.
         os_signpost_interval_end(getLogger(), getSignpostForObject(O),
                                  "LLVM Timers", "");
       }
     }
   }

 void SignpostEmitter::startInterval(const void *O, StringRef Name) {
 #if HAVE_ANY_SIGNPOST_IMPL
   if (Impl == nullptr)
     return;
   return Impl->startInterval(O, Name);
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }
 
-void SignpostEmitter::endInterval(const void *O, StringRef Name) {
+#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) {
 #if HAVE_ANY_SIGNPOST_IMPL
   if (Impl == nullptr)
     return;
-  Impl->endInterval(O, Name);
+  Impl->endInterval(O);
 #endif // if !HAVE_ANY_SIGNPOST_IMPL
 }

llvm/lib/Support/Timer.cpp

void Timer::startTimer() {
   StartTime = TimeRecord::getCurrentTime(true);
 }
 
 void Timer::stopTimer() {
   assert(Running && "Cannot stop a paused timer");
   Running = false;
   Time += TimeRecord::getCurrentTime(false);
   Time -= StartTime;
-  Signposts->endInterval(this, getName());
+  Signposts->endInterval(this);
 }
 
 void Timer::clear() {
   Running = Triggered = false;
   Time = StartTime = TimeRecord();
 }
 
 static void printVal(double Val, double Total, raw_ostream &OS) {