Make BinaryStreamReader::readCString a bit faster.

[zturner]

Summary

Previously it would do a character by character search for a null terminator, to account for the fact that an arbitrary stream need not store its data contiguously so you couldn't just do a memchr. However, the stream API has a function which will return the longest contiguous chunk without doing a copy, and by using this function we can do a memchr on the individual chunks. For certain types of streams like data from object files etc, this is guaranteed to find the null terminator with only a single memchr, but even with discontiguous streams such as MappedBlockStream, it's rare that any given string will cross a block boundary, so even those will almost always be satisfied with a single memchr.

This optimization is worth a 10-12% reduction in link time (4.2 seconds -> 3.75 seconds)

[rnk]

Hm, did my comment not make it through?

llvm/lib/Support/BinaryStreamReader.cpp

52–54

How about S = S.take_front(strnlen(S.data(), S.size()))? There are some neat tricks for making strlen fast that don't work for memchr.

llvm/lib/Support/BinaryStreamReader.cpp

 Error BinaryStreamReader::readBytes(ArrayRef<uint8_t> &Buffer, uint32_t Size) {
   if (auto EC = Stream.readBytes(Offset, Size, Buffer))
     return EC;
   Offset += Size;
   return Error::success();
 }
 
 Error BinaryStreamReader::readCString(StringRef &Dest) {
-  // TODO: This could be made more efficient by using readLongestContiguousChunk
-  // and searching for null terminators in the resulting buffer.
-
-  uint32_t Length = 0;
-  // First compute the length of the string by reading 1 byte at a time.
   uint32_t OriginalOffset = getOffset();
-  const char *C;
+  uint32_t FoundOffset = 0;
   while (true) {
-    if (auto EC = readObject(C))
+    uint32_t ThisOffset = getOffset();
+    ArrayRef<uint8_t> Buffer;
+    if (auto EC = readLongestContiguousChunk(Buffer))
       return EC;
-    if (*C == '\0')
+    StringRef S(reinterpret_cast<const char *>(Buffer.begin()), Buffer.size());
+    size_t Pos = S.find_first_of('\0');
+    if (LLVM_LIKELY(Pos != StringRef::npos)) {
+      FoundOffset = Pos + ThisOffset;
       break;
-    ++Length;
     }
-  // Now go back and request a reference for that many bytes.
-  uint32_t NewOffset = getOffset();
+  }
+  assert(FoundOffset >= OriginalOffset);
+
   setOffset(OriginalOffset);
+  size_t Length = FoundOffset - OriginalOffset;
 
   if (auto EC = readFixedString(Dest, Length))
     return EC;
 
-  // Now set the offset back to where it was after we calculated the length.
-  setOffset(NewOffset);
+  // Now set the offset back to after the null terminator.
+  setOffset(FoundOffset + 1);
   return Error::success();
 }
 
 Error BinaryStreamReader::readFixedString(StringRef &Dest, uint32_t Length) {
   ArrayRef<uint8_t> Bytes;
   if (auto EC = readBytes(Bytes, Length))
     return EC;
   Dest = StringRef(reinterpret_cast<const char *>(Bytes.begin()), Bytes.size());
BinaryStreamReader::split(uint32_t Off) const {
   BinaryStreamRef First = Stream.drop_front(Offset);
 
   BinaryStreamRef Second = First.drop_front(Off);
   First = First.keep_front(Off);
   BinaryStreamReader W1{First};
   BinaryStreamReader W2{Second};
   return std::make_pair(W1, W2);
 }
 No newline at end of file