[Object][Archive] Improve performance.
authorMichael J. Spencer <bigcheesegs@gmail.com>
Sun, 3 Feb 2013 10:48:50 +0000 (10:48 +0000)
committerMichael J. Spencer <bigcheesegs@gmail.com>
Sun, 3 Feb 2013 10:48:50 +0000 (10:48 +0000)
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
lib/Object/Archive.cpp
tools/llvm-nm/llvm-nm.cpp

index 8046efda8e4cbdc9edbdb3c47e2250092dad5ef9..e2478f6754b0900164b16d1639b2a6f98fa497df 100644 (file)
 #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;
   };
 
index e1433384bcae19b9639bf2be577581bfe6f16434..0e13d0540fa60f6a265ab6bcbfb91a6cfac8a306 100644 (file)
@@ -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);
index 056fd3542289b847083906c5c166bc9aae23cc10..a24aae6061a4037e5cbab3c3428fbbf4ee071142 100644 (file)
@@ -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);