Cache memory buffer's name length

[serge-sans-paille]

Summary

This avoids repeated calls to strlen while we already know its value.

When preprocessing sqlite3.c, this gives a surprising 2% speedup.

Full benchmark available here: https://llvm-compile-time-tracker.com/compare.php?from=5279e6a7d677cdf4488883b77aacab911318100c&to=389601b0dbdf23cf25167ddfc49b3af5742ebd9a&stat=instructions:u

[serge-sans-paille]

gentle ping :-)

[fhahn]

Unfortunately it looks like this commit triggers an assertion when building Clang on macOS. I reverted the commit for now with more details about the crash in the commit message.

[fhahn]

(see also https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/32962/console)

[ronlieb]

also breaks amdgpu hip and openmp buildbots

[thakis]

Thanks for the quick revert!

Should be very easy to repro by just building clang, or llvm-mt, or any target that depends on anything tablegen'd on any platform (linux: http://45.33.8.238/linux/92677/step_4.txt win: http://45.33.8.238/win/70883/step_4.txt)

llvm/lib/Support/MemoryBuffer.cpp

void MemoryBuffer::init(const char *BufStart, const char *BufEnd,
   BufferStart = BufStart;
   BufferEnd = BufEnd;
 }
 
 //===----------------------------------------------------------------------===//
 // MemoryBufferMem implementation.
 //===----------------------------------------------------------------------===//
 
-/// CopyStringRef - Copies contents of a StringRef into a block of memory and
-/// null-terminates it.
+/// CopyStringRef - Copies contents of a StringRef into a block of memory.
 static void CopyStringRef(char *Memory, StringRef Data) {
   if (!Data.empty())
     memcpy(Memory, Data.data(), Data.size());
-  Memory[Data.size()] = 0; // Null terminate string.
 }
 
 namespace {
 struct NamedBufferAlloc {
   const Twine &Name;
   NamedBufferAlloc(const Twine &Name) : Name(Name) {}
 };
 } // namespace
 
 void *operator new(size_t N, const NamedBufferAlloc &Alloc) {
   SmallString<256> NameBuf;
   StringRef NameRef = Alloc.Name.toStringRef(NameBuf);
 
-  char *Mem = static_cast<char *>(operator new(N + NameRef.size() + 1));
-  CopyStringRef(Mem + N, NameRef);
+  char *Mem =
+      static_cast<char *>(operator new(N + sizeof(size_t) + NameRef.size()));
+  *reinterpret_cast<size_t *>(Mem + N) = NameRef.size();
+  CopyStringRef(Mem + N + sizeof(size_t), NameRef);
   return Mem;
 }
 
 namespace {
 /// MemoryBufferMem - Named MemoryBuffer pointing to a block of memory.
 template<typename MB>
 class MemoryBufferMem : public MB {
 public:
   MemoryBufferMem(StringRef InputData, bool RequiresNullTerminator) {
     MemoryBuffer::init(InputData.begin(), InputData.end(),
                        RequiresNullTerminator);
   }
 
   /// Disable sized deallocation for MemoryBufferMem, because it has
   /// tail-allocated data.
   void operator delete(void *p) { ::operator delete(p); }
 
   StringRef getBufferIdentifier() const override {
     // The name is stored after the class itself.
-    return StringRef(reinterpret_cast<const char *>(this + 1));
+    return StringRef(reinterpret_cast<const char *>(this + 1) + sizeof(size_t),
+                     *reinterpret_cast<const size_t *>(this + 1));
   }
 
   MemoryBuffer::BufferKind getBufferKind() const override {
     return MemoryBuffer::MemoryBuffer_Malloc;
   }
 };
 } // namespace
 
public:
   }
 
   /// Disable sized deallocation for MemoryBufferMMapFile, because it has
   /// tail-allocated data.
   void operator delete(void *p) { ::operator delete(p); }
 
   StringRef getBufferIdentifier() const override {
     // The name is stored after the class itself.
-    return StringRef(reinterpret_cast<const char *>(this + 1));
+    return StringRef(reinterpret_cast<const char *>(this + 1) + sizeof(size_t),
+                     *reinterpret_cast<const size_t *>(this + 1));
   }
 
   MemoryBuffer::BufferKind getBufferKind() const override {
     return MemoryBuffer::MemoryBuffer_MMap;
   }
 
   void dontNeedIfMmap() override { MFR.dontNeed(); }
 };
WritableMemoryBuffer::getNewUninitMemBuffer(size_t Size,
 
   // Use 16-byte alignment if no alignment is specified.
   Align BufAlign = Alignment.value_or(Align(16));
 
   // Allocate space for the MemoryBuffer, the data and the name. It is important
   // that MemoryBuffer and data are aligned so PointerIntPair works with them.
   SmallString<256> NameBuf;
   StringRef NameRef = BufferName.toStringRef(NameBuf);
-  size_t StringLen = sizeof(MemBuffer) + NameRef.size() + 1;
+
+  size_t StringLen = sizeof(MemBuffer) + sizeof(size_t) + NameRef.size();
   size_t RealLen = StringLen + Size + 1 + BufAlign.value();
   if (RealLen <= Size) // Check for rollover.
     return nullptr;
   char *Mem = static_cast<char*>(operator new(RealLen, std::nothrow));
   if (!Mem)
     return nullptr;
 
   // The name is stored after the class itself.
-  CopyStringRef(Mem + sizeof(MemBuffer), NameRef);
+  *reinterpret_cast<size_t *>(Mem + sizeof(MemBuffer)) =
+      NameRef.size(); // Null terminate buffer.
+  CopyStringRef(Mem + sizeof(MemBuffer) + sizeof(size_t), NameRef);
 
   // The buffer begins after the name and must be aligned.
   char *Buf = (char *)alignAddr(Mem + StringLen, BufAlign);
-  Buf[Size] = 0; // Null terminate buffer.
 
   auto *Ret = new (Mem) MemBuffer(StringRef(Buf, Size), true);
+  Buf[Size] = 0; // Null terminate buffer.
   return std::unique_ptr<WritableMemoryBuffer>(Ret);
 }
 
 std::unique_ptr<WritableMemoryBuffer>
 WritableMemoryBuffer::getNewMemBuffer(size_t Size, const Twine &BufferName) {
   auto SB = WritableMemoryBuffer::getNewUninitMemBuffer(Size, BufferName);
   if (!SB)
     return nullptr;