[lld] 0db402c - [lld] Buffer writes when composing a single diagnostic

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2021-09-09T09:27:14-07:00
New Revision: 0db402c5b4ef60ca8d20dda8bb59c7288a534e2a

URL: https://github.com/llvm/llvm-project/commit/0db402c5b4ef60ca8d20dda8bb59c7288a534e2a
DIFF: https://github.com/llvm/llvm-project/commit/0db402c5b4ef60ca8d20dda8bb59c7288a534e2a.diff

LOG: [lld] Buffer writes when composing a single diagnostic

llvm::errs() is unbuffered. On a POSIX platform, composing a diagnostic
string may invoke the ::write syscall multiple times, which can be slow.
Buffer writes to a temporary SmallString when composing a single diagnostic to
reduce the number of ::write syscalls to one (also easier to read under
strace/truss).

For an invocation of ld.lld with 62000+ lines of
`ld.lld: warning: symbol ordering file: no such symbol: ` warnings (D87121),
the buffering decreases the write time from 1s to 0.4s (for /dev/tty) and
from 0.4s to 0.1s (for a tmpfs file). This can speed up
`relocation R_X86_64_PC32 out of range` diagnostic printing as well
with `--noinhibit-exec --no-fatal-warnings`.

Reviewed By: jhenderson

Differential Revision: https://reviews.llvm.org/D87272

Added: 
    

Modified: 
    lld/Common/ErrorHandler.cpp
    lld/include/lld/Common/ErrorHandler.h
    llvm/include/llvm/Support/raw_ostream.h

Removed: 
    


################################################################################
diff  --git a/lld/Common/ErrorHandler.cpp b/lld/Common/ErrorHandler.cpp
index 269a0f62ec658..c8c5ffed71268 100644
--- a/lld/Common/ErrorHandler.cpp
+++ b/lld/Common/ErrorHandler.cpp
@@ -168,11 +168,28 @@ std::string ErrorHandler::getLocation(const Twine &msg) {
   return std::string(logName);
 }
 
+void ErrorHandler::reportDiagnostic(StringRef location, Colors c,
+                                    StringRef diagKind, const Twine &msg) {
+  SmallString<256> buf;
+  raw_svector_ostream os(buf);
+  os << sep << location << ": ";
+  if (!diagKind.empty()) {
+    if (lld::errs().colors_enabled()) {
+      os.enable_colors(true);
+      os << c << diagKind << ": " << Colors::RESET;
+    } else {
+      os << diagKind << ": ";
+    }
+  }
+  os << msg << '\n';
+  lld::errs() << buf;
+}
+
 void ErrorHandler::log(const Twine &msg) {
   if (!verbose || disableOutput)
     return;
   std::lock_guard<std::mutex> lock(mu);
-  lld::errs() << logName << ": " << msg << "\n";
+  reportDiagnostic(logName, Colors::RESET, "", msg);
 }
 
 void ErrorHandler::message(const Twine &msg) {
@@ -190,8 +207,7 @@ void ErrorHandler::warn(const Twine &msg) {
   }
 
   std::lock_guard<std::mutex> lock(mu);
-  lld::errs() << sep << getLocation(msg) << ": " << Colors::MAGENTA
-              << "warning: " << Colors::RESET << msg << "\n";
+  reportDiagnostic(getLocation(msg), Colors::MAGENTA, "warning", msg);
   sep = getSeparator(msg);
 }
 
@@ -217,12 +233,9 @@ void ErrorHandler::error(const Twine &msg) {
     std::lock_guard<std::mutex> lock(mu);
 
     if (errorLimit == 0 || errorCount < errorLimit) {
-      lld::errs() << sep << getLocation(msg) << ": " << Colors::RED
-                  << "error: " << Colors::RESET << msg << "\n";
+      reportDiagnostic(getLocation(msg), Colors::RED, "error", msg);
     } else if (errorCount == errorLimit) {
-      lld::errs() << sep << getLocation(msg) << ": " << Colors::RED
-                  << "error: " << Colors::RESET << errorLimitExceededMsg
-                  << "\n";
+      reportDiagnostic(logName, Colors::RED, "error", errorLimitExceededMsg);
       exit = exitEarly;
     }
 

diff  --git a/lld/include/lld/Common/ErrorHandler.h b/lld/include/lld/Common/ErrorHandler.h
index 95d92f3594ba6..04602fad6d516 100644
--- a/lld/include/lld/Common/ErrorHandler.h
+++ b/lld/include/lld/Common/ErrorHandler.h
@@ -124,6 +124,8 @@ class ErrorHandler {
   using Colors = raw_ostream::Colors;
 
   std::string getLocation(const Twine &msg);
+  void reportDiagnostic(StringRef location, Colors c, StringRef diagKind,
+                        const Twine &msg);
 };
 
 /// Returns the default error handler.

diff  --git a/llvm/include/llvm/Support/raw_ostream.h b/llvm/include/llvm/Support/raw_ostream.h
index c669c2babad9e..00d9c7db33b60 100644
--- a/llvm/include/llvm/Support/raw_ostream.h
+++ b/llvm/include/llvm/Support/raw_ostream.h
@@ -330,6 +330,8 @@ class raw_ostream {
   // changeColor() has no effect until enable_colors(true) is called.
   virtual void enable_colors(bool enable) { ColorEnabled = enable; }
 
+  bool colors_enabled() const { return ColorEnabled; }
+
   /// Tie this stream to the specified stream. Replaces any existing tied-to
   /// stream. Specifying a nullptr unties the stream.
   void tie(raw_ostream *TieTo) { TiedStream = TieTo; }