[llvm] 5ef2cb3 - [FormatVariadic] Reduce allocations

[Benjamin Kramer via llvm-commits]

Author: Benjamin Kramer
Date: 2020-04-11T14:54:32+02:00
New Revision: 5ef2cb3df4ce261ec0ec647f38495d2bf32b996e

URL: https://github.com/llvm/llvm-project/commit/5ef2cb3df4ce261ec0ec647f38495d2bf32b996e
DIFF: https://github.com/llvm/llvm-project/commit/5ef2cb3df4ce261ec0ec647f38495d2bf32b996e.diff

LOG: [FormatVariadic] Reduce allocations

- Move Adapters array to the stack, we know the size precisely
- Parse format string on demand into a SmallVector. In theory this could
  lead to parsing it multiple times, but I couldn't find a single instance
  of that in LLVM.
- Make more of the implementation details private.

Added: 
    

Modified: 
    llvm/include/llvm/Support/FormatVariadic.h
    llvm/lib/Support/FormatVariadic.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/FormatVariadic.h b/llvm/include/llvm/Support/FormatVariadic.h
index 86a9d30cc138..60ea07506838 100644
--- a/llvm/include/llvm/Support/FormatVariadic.h
+++ b/llvm/include/llvm/Support/FormatVariadic.h
@@ -25,6 +25,7 @@
 #ifndef LLVM_SUPPORT_FORMATVARIADIC_H
 #define LLVM_SUPPORT_FORMATVARIADIC_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -63,23 +64,8 @@ struct ReplacementItem {
 
 class formatv_object_base {
 protected:
-  // The parameters are stored in a std::tuple, which does not provide runtime
-  // indexing capabilities.  In order to enable runtime indexing, we use this
-  // structure to put the parameters into a std::vector.  Since the parameters
-  // are not all the same type, we use some type-erasure by wrapping the
-  // parameters in a template class that derives from a non-template superclass.
-  // Essentially, we are converting a std::tuple<Derived<Ts...>> to a
-  // std::vector<Base*>.
-  struct create_adapters {
-    template <typename... Ts>
-    std::vector<detail::format_adapter *> operator()(Ts &... Items) {
-      return std::vector<detail::format_adapter *>{&Items...};
-    }
-  };
-
   StringRef Fmt;
-  std::vector<detail::format_adapter *> Adapters;
-  std::vector<ReplacementItem> Replacements;
+  ArrayRef<detail::format_adapter *> Adapters;
 
   static bool consumeFieldLayout(StringRef &Spec, AlignStyle &Where,
                                  size_t &Align, char &Pad);
@@ -87,23 +73,16 @@ class formatv_object_base {
   static std::pair<ReplacementItem, StringRef>
   splitLiteralAndReplacement(StringRef Fmt);
 
-public:
-  formatv_object_base(StringRef Fmt, std::size_t ParamCount)
-      : Fmt(Fmt), Replacements(parseFormatString(Fmt)) {
-    Adapters.reserve(ParamCount);
-  }
+  formatv_object_base(StringRef Fmt,
+                      ArrayRef<detail::format_adapter *> Adapters)
+      : Fmt(Fmt), Adapters(Adapters) {}
 
   formatv_object_base(formatv_object_base const &rhs) = delete;
+  formatv_object_base(formatv_object_base &&rhs) = default;
 
-  formatv_object_base(formatv_object_base &&rhs)
-      : Fmt(std::move(rhs.Fmt)),
-        Adapters(), // Adapters are initialized by formatv_object
-        Replacements(std::move(rhs.Replacements)) {
-    Adapters.reserve(rhs.Adapters.size());
-  };
-
+public:
   void format(raw_ostream &S) const {
-    for (auto &R : Replacements) {
+    for (auto &R : parseFormatString(Fmt)) {
       if (R.Type == ReplacementType::Empty)
         continue;
       if (R.Type == ReplacementType::Literal) {
@@ -121,7 +100,7 @@ class formatv_object_base {
       Align.format(S, R.Options);
     }
   }
-  static std::vector<ReplacementItem> parseFormatString(StringRef Fmt);
+  static SmallVector<ReplacementItem, 2> parseFormatString(StringRef Fmt);
 
   static Optional<ReplacementItem> parseReplacementItem(StringRef Spec);
 
@@ -150,12 +129,29 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
   // of the parameters, we have to own the storage for the parameters here, and
   // have the base class store type-erased pointers into this tuple.
   Tuple Parameters;
+  std::array<detail::format_adapter *, std::tuple_size<Tuple>::value>
+      ParameterPointers;
+
+  // The parameters are stored in a std::tuple, which does not provide runtime
+  // indexing capabilities.  In order to enable runtime indexing, we use this
+  // structure to put the parameters into a std::array.  Since the parameters
+  // are not all the same type, we use some type-erasure by wrapping the
+  // parameters in a template class that derives from a non-template superclass.
+  // Essentially, we are converting a std::tuple<Derived<Ts...>> to a
+  // std::array<Base*>.
+  struct create_adapters {
+    template <typename... Ts>
+    std::array<detail::format_adapter *, std::tuple_size<Tuple>::value>
+    operator()(Ts &... Items) {
+      return {&Items...};
+    }
+  };
 
 public:
   formatv_object(StringRef Fmt, Tuple &&Params)
-      : formatv_object_base(Fmt, std::tuple_size<Tuple>::value),
+      : formatv_object_base(Fmt, ParameterPointers),
         Parameters(std::move(Params)) {
-    Adapters = apply_tuple(create_adapters(), Parameters);
+    ParameterPointers = apply_tuple(create_adapters(), Parameters);
   }
 
   formatv_object(formatv_object const &rhs) = delete;
@@ -163,7 +159,8 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
   formatv_object(formatv_object &&rhs)
       : formatv_object_base(std::move(rhs)),
         Parameters(std::move(rhs.Parameters)) {
-    Adapters = apply_tuple(create_adapters(), Parameters);
+    ParameterPointers = apply_tuple(create_adapters(), Parameters);
+    Adapters = ParameterPointers;
   }
 };
 
@@ -249,9 +246,7 @@ template <typename Tuple> class formatv_object : public formatv_object_base {
 // assertion.  Otherwise, it will try to do something reasonable, but in general
 // the details of what that is are undefined.
 //
-template <typename... Ts>
-inline auto formatv(const char *Fmt, Ts &&... Vals) -> formatv_object<decltype(
-    std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...))> {
+template <typename... Ts> inline auto formatv(const char *Fmt, Ts &&... Vals) {
   using ParamTuple = decltype(
       std::make_tuple(detail::build_format_adapter(std::forward<Ts>(Vals))...));
   return formatv_object<ParamTuple>(

diff  --git a/llvm/lib/Support/FormatVariadic.cpp b/llvm/lib/Support/FormatVariadic.cpp
index 0d61fae22323..632e879e540d 100644
--- a/llvm/lib/Support/FormatVariadic.cpp
+++ b/llvm/lib/Support/FormatVariadic.cpp
@@ -141,9 +141,9 @@ formatv_object_base::splitLiteralAndReplacement(StringRef Fmt) {
   return std::make_pair(ReplacementItem{Fmt}, StringRef());
 }
 
-std::vector<ReplacementItem>
+SmallVector<ReplacementItem, 2>
 formatv_object_base::parseFormatString(StringRef Fmt) {
-  std::vector<ReplacementItem> Replacements;
+  SmallVector<ReplacementItem, 2> Replacements;
   ReplacementItem I;
   while (!Fmt.empty()) {
     std::tie(I, Fmt) = splitLiteralAndReplacement(Fmt);