[Object][Archive] Improve performance.

Improve performance of iterating over children and accessing the member file
buffer by caching the file size and moving code out to the header.

This also makes getBuffer return a StringRef instead of a MemoryBuffer. Both
fixing a memory leak and removing a malloc.

This takes getBuffer from ~10% of the time in lld to unmeasurable.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@174272 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/Object/Archive.h

@@ -14,22 +14,78 @@
 #ifndef LLVM_OBJECT_ARCHIVE_H
 #define LLVM_OBJECT_ARCHIVE_H
 
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 namespace llvm {
 namespace object {
+struct ArchiveMemberHeader {
+  char Name[16];
+  char LastModified[12];
+  char UID[6];
+  char GID[6];
+  char AccessMode[8];
+  char Size[10]; ///< Size of data, not including header or padding.
+  char Terminator[2];
+
+  ///! Get the name without looking up long names.
+  llvm::StringRef getName() const {
+    char EndCond;
+    if (Name[0] == '/' || Name[0] == '#')
+      EndCond = ' ';
+    else
+      EndCond = '/';
+    llvm::StringRef::size_type end =
+        llvm::StringRef(Name, sizeof(Name)).find(EndCond);
+    if (end == llvm::StringRef::npos)
+      end = sizeof(Name);
+    assert(end <= sizeof(Name) && end > 0);
+    // Don't include the EndCond if there is one.
+    return llvm::StringRef(Name, end);
+  }
+
+  uint64_t getSize() const {
+    uint64_t ret;
+    if (llvm::StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, ret))
+      llvm_unreachable("Size is not an integer.");
+    return ret;
+  }
+};
+
+static const ArchiveMemberHeader *ToHeader(const char *base) {
+  return reinterpret_cast<const ArchiveMemberHeader *>(base);
+}
 
 class Archive : public Binary {
   virtual void anchor();
 public:
   class Child {
     const Archive *Parent;
+    /// \brief Includes header but not padding byte.
     StringRef Data;
+    /// \brief Offset from Data to the start of the file.
+    uint16_t StartOfFile;
 
   public:
-    Child(const Archive *p, StringRef d) : Parent(p), Data(d) {}
+    Child(const Archive *p, StringRef d) : Parent(p), Data(d) {
+      if (!p || d.empty())
+        return;
+      // Setup StartOfFile and PaddingBytes.
+      StartOfFile = sizeof(ArchiveMemberHeader);
+      // Don't include attached name.
+      StringRef Name = ToHeader(Data.data())->getName();
+      if (Name.startswith("#1/")) {
+        uint64_t NameSize;
+        if (Name.substr(3).rtrim(" ").getAsInteger(10, NameSize))
+          llvm_unreachable("Long name length is not an integer");
+        StartOfFile += NameSize;
+      }
+    }
 
     bool operator ==(const Child &other) const {
       return (Parent == other.Parent) && (Data.begin() == other.Data.begin());
@@ -39,16 +95,48 @@ public:
       return Data.begin() < other.Data.begin();
     }
 
-    Child getNext() const;
+    Child getNext() const {
+      size_t SpaceToSkip = Data.size();
+      // If it's odd, add 1 to make it even.
+      if (SpaceToSkip & 1)
+        ++SpaceToSkip;
+
+      const char *NextLoc = Data.data() + SpaceToSkip;
+
+      // Check to see if this is past the end of the archive.
+      if (NextLoc >= Parent->Data->getBufferEnd())
+        return Child(Parent, StringRef(0, 0));
+
+      size_t NextSize =
+          sizeof(ArchiveMemberHeader) + ToHeader(NextLoc)->getSize();
+
+      return Child(Parent, StringRef(NextLoc, NextSize));
+    }
+
     error_code getName(StringRef &Result) const;
     int getLastModified() const;
     int getUID() const;
     int getGID() const;
     int getAccessMode() const;
-    ///! Return the size of the archive member without the header or padding.
-    uint64_t getSize() const;
+    /// \return the size of the archive member without the header or padding.
+    uint64_t getSize() const { return Data.size() - StartOfFile; }
+
+    StringRef getBuffer() const {
+      return StringRef(Data.data() + StartOfFile, getSize());
+    }
+
+    error_code getMemoryBuffer(OwningPtr<MemoryBuffer> &Result,
+                               bool FullPath = false) const {
+      StringRef Name;
+      if (error_code ec = getName(Name))
+        return ec;
+      SmallString<128> Path;
+      Result.reset(MemoryBuffer::getMemBuffer(
+          getBuffer(), FullPath ? (Twine(Parent->getFileName()) + "(" + Name +
+                                   ")").toStringRef(Path) : Name, false));
+      return error_code::success();
+    }
 
-    MemoryBuffer *getBuffer() const;
     error_code getAsBinary(OwningPtr<Binary> &Result) const;
   };
 

lib/Object/Archive.cpp

@@ -14,7 +14,6 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/Support/Endian.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MemoryBuffer.h"
 
 using namespace llvm;
@@ -22,45 +21,6 @@ using namespace object;
 
 static const char *Magic = "!<arch>\n";
 
-namespace {
-struct ArchiveMemberHeader {
-  char Name[16];
-  char LastModified[12];
-  char UID[6];
-  char GID[6];
-  char AccessMode[8];
-  char Size[10]; ///< Size of data, not including header or padding.
-  char Terminator[2];
-
-  ///! Get the name without looking up long names.
-  StringRef getName() const {
-    char EndCond;
-    if (Name[0] == '/' || Name[0] == '#')
-      EndCond = ' ';
-    else
-      EndCond = '/';
-    StringRef::size_type end = StringRef(Name, sizeof(Name)).find(EndCond);
-    if (end == StringRef::npos)
-      end = sizeof(Name);
-    assert(end <= sizeof(Name) && end > 0);
-    // Don't include the EndCond if there is one.
-    return StringRef(Name, end);
-  }
-
-  uint64_t getSize() const {
-    uint64_t ret;
-    if (StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, ret))
-      llvm_unreachable("Size is not an integer.");
-    return ret;
-  }
-};
-}
-
-static const ArchiveMemberHeader *ToHeader(const char *base) {
-  return reinterpret_cast<const ArchiveMemberHeader *>(base);
-}
-
-
 static bool isInternalMember(const ArchiveMemberHeader &amh) {
   static const char *const internals[] = {
     "/",
@@ -78,25 +38,6 @@ static bool isInternalMember(const ArchiveMemberHeader &amh) {
 
 void Archive::anchor() { }
 
-Archive::Child Archive::Child::getNext() const {
-  size_t SpaceToSkip = sizeof(ArchiveMemberHeader) +
-    ToHeader(Data.data())->getSize();
-  // If it's odd, add 1 to make it even.
-  if (SpaceToSkip & 1)
-    ++SpaceToSkip;
-
-  const char *NextLoc = Data.data() + SpaceToSkip;
-
-  // Check to see if this is past the end of the archive.
-  if (NextLoc >= Parent->Data->getBufferEnd())
-    return Child(Parent, StringRef(0, 0));
-
-  size_t NextSize = sizeof(ArchiveMemberHeader) +
-    ToHeader(NextLoc)->getSize();
-
-  return Child(Parent, StringRef(NextLoc, NextSize));
-}
-
 error_code Archive::Child::getName(StringRef &Result) const {
   StringRef name = ToHeader(Data.data())->getName();
   // Check if it's a special name.
@@ -149,39 +90,12 @@ error_code Archive::Child::getName(StringRef &Result) const {
   return object_error::success;
 }
 
-uint64_t Archive::Child::getSize() const {
-  uint64_t size = ToHeader(Data.data())->getSize();
-  // Don't include attached name.
-  StringRef name =  ToHeader(Data.data())->getName();
-  if (name.startswith("#1/")) {
-    uint64_t name_size;
-    if (name.substr(3).rtrim(" ").getAsInteger(10, name_size))
-      llvm_unreachable("Long name length is not an integer");
-    size -= name_size;
-  }
-  return size;
-}
-
-MemoryBuffer *Archive::Child::getBuffer() const {
-  StringRef name = ToHeader(Data.data())->getName();
-  int size = sizeof(ArchiveMemberHeader);
-  if (name.startswith("#1/")) {
-    uint64_t name_size;
-    if (name.substr(3).rtrim(" ").getAsInteger(10, name_size))
-      llvm_unreachable("Long name length is not an integer");
-    size += name_size;
-  }
-  if (getName(name))
-    return 0;
-  return MemoryBuffer::getMemBuffer(Data.substr(size, getSize()),
-                                    name,
-                                    false);
-}
-
 error_code Archive::Child::getAsBinary(OwningPtr<Binary> &Result) const {
   OwningPtr<Binary> ret;
-  if (error_code ec =
-    createBinary(getBuffer(), ret))
+  OwningPtr<MemoryBuffer> Buff;
+  if (error_code ec = getMemoryBuffer(Buff))
+    return ec;
+  if (error_code ec = createBinary(Buff.take(), ret))
     return ec;
   Result.swap(ret);
   return object_error::success;
@@ -270,13 +184,12 @@ Archive::child_iterator Archive::end_children() const {
 }
 
 error_code Archive::Symbol::getName(StringRef &Result) const {
-  Result =
-    StringRef(Parent->SymbolTable->getBuffer()->getBufferStart() + StringIndex);
+  Result = StringRef(Parent->SymbolTable->getBuffer().begin() + StringIndex);
   return object_error::success;
 }
 
 error_code Archive::Symbol::getMember(child_iterator &Result) const {
-  const char *Buf = Parent->SymbolTable->getBuffer()->getBufferStart();
+  const char *Buf = Parent->SymbolTable->getBuffer().begin();
   const char *Offsets = Buf + 4;
   uint32_t Offset = 0;
   if (Parent->kind() == K_GNU) {
@@ -326,13 +239,13 @@ Archive::Symbol Archive::Symbol::getNext() const {
   Symbol t(*this);
   // Go to one past next null.
   t.StringIndex =
-    Parent->SymbolTable->getBuffer()->getBuffer().find('\0', t.StringIndex) + 1;
+      Parent->SymbolTable->getBuffer().find('\0', t.StringIndex) + 1;
   ++t.SymbolIndex;
   return t;
 }
 
 Archive::symbol_iterator Archive::begin_symbols() const {
-  const char *buf = SymbolTable->getBuffer()->getBufferStart();
+  const char *buf = SymbolTable->getBuffer().begin();
   if (kind() == K_GNU) {
     uint32_t symbol_count = 0;
     symbol_count = *reinterpret_cast<const support::ubig32_t*>(buf);
@@ -347,13 +260,12 @@ Archive::symbol_iterator Archive::begin_symbols() const {
     symbol_count = *reinterpret_cast<const support::ulittle32_t*>(buf);
     buf += 4 + (symbol_count * 2); // Skip indices.
   }
-  uint32_t string_start_offset =
-    buf - SymbolTable->getBuffer()->getBufferStart();
+  uint32_t string_start_offset = buf - SymbolTable->getBuffer().begin();
   return symbol_iterator(Symbol(this, 0, string_start_offset));
 }
 
 Archive::symbol_iterator Archive::end_symbols() const {
-  const char *buf = SymbolTable->getBuffer()->getBufferStart();
+  const char *buf = SymbolTable->getBuffer().begin();
   uint32_t symbol_count = 0;
   if (kind() == K_GNU) {
     symbol_count = *reinterpret_cast<const support::ubig32_t*>(buf);

tools/llvm-nm/llvm-nm.cpp

@@ -384,7 +384,9 @@ static void DumpSymbolNamesFromFile(std::string &Filename) {
         OwningPtr<Binary> child;
         if (i->getAsBinary(child)) {
           // Try opening it as a bitcode file.
-          OwningPtr<MemoryBuffer> buff(i->getBuffer());
+          OwningPtr<MemoryBuffer> buff;
+          if (error(i->getMemoryBuffer(buff)))
+            return;
           Module *Result = 0;
           if (buff)
             Result = ParseBitcodeFile(buff.get(), Context, &ErrorMessage);