Let ElfCache be unbounded

Summary:
[Folly] Let `ElfCache` be unbounded to permit binaries with many dynamically-linked libraries from continuously missing the cache when symbolizing.

Some applications symbolize stacktraces relatively frequently. When the build is using dynamic linking, such as might be done when developing the application or running the unit-tests, the overhead from re-loading `ElfFile` instances can appear. Address this by making the cache be unboundedly large.

This is unlikely to affect applications built using mostly-static linking, such as might be done for production builds, even when they are large applications built from many libraries, since the capacity parameter is usually much larger than the total number of dynamically loaded libraries.

Currently, no substantial work is done when initializing `ElfFile` instances. If substantial work were to be done, especially in an unoptimized build such as might be used when developing the application or running the unit-tests, and instances do not stay in cache, then that substantial work would be repeated whenever symbolizing stacktraces rather than being amortized.

Reviewed By: luciang

Differential Revision: D18819555

fbshipit-source-id: 0e75326133e901e7bd49ea3cefe9b318e6d4466f

folly/detail/SingletonStackTrace.cpp

@@ -36,8 +36,7 @@ std::string getSingletonStackTrace() {
   if (!getStackTraceSafe(*addresses)) {
     return "";
   } else {
-    constexpr size_t kDefaultCapacity = 500;
-    symbolizer::ElfCache elfCache(kDefaultCapacity);
+    symbolizer::ElfCache elfCache;
 
     symbolizer::Symbolizer symbolizer(&elfCache);
     symbolizer.symbolize(*addresses);

folly/experimental/symbolizer/ElfCache.cpp

@@ -94,17 +94,13 @@ std::shared_ptr<ElfFile> SignalSafeElfCache::getFile(StringPiece p) {
   return pos->file;
 }
 
-ElfCache::ElfCache(size_t capacity) : capacity_(capacity) {}
-
 std::shared_ptr<ElfFile> ElfCache::getFile(StringPiece p) {
   std::lock_guard<std::mutex> lock(mutex_);
 
   auto pos = files_.find(p);
   if (pos != files_.end()) {
-    // Found, move to back (MRU)
+    // Found
     auto& entry = pos->second;
-    lruList_.erase(lruList_.iterator_to(*entry));
-    lruList_.push_back(*entry);
     return filePtr(entry);
   }
 
@@ -118,16 +114,9 @@ std::shared_ptr<ElfFile> ElfCache::getFile(StringPiece p) {
     return nullptr;
   }
 
-  if (files_.size() == capacity_) {
-    auto& e = lruList_.front();
-    lruList_.pop_front();
-    files_.erase(e.path);
-  }
-
-  files_.emplace(entry->path, entry);
-  lruList_.push_back(*entry);
+  pos = files_.emplace(path, std::move(entry)).first;
 
-  return filePtr(entry);
+  return filePtr(pos->second);
 }
 
 std::shared_ptr<ElfFile> ElfCache::filePtr(const std::shared_ptr<Entry>& e) {

folly/experimental/symbolizer/ElfCache.h

@@ -23,7 +23,6 @@
 #include <unordered_map>
 
 #include <boost/intrusive/avl_set.hpp>
-#include <boost/intrusive/list.hpp>
 
 #include <folly/Range.h>
 #include <folly/experimental/symbolizer/Elf.h>
@@ -91,32 +90,19 @@ class SignalSafeElfCache : public ElfCacheBase {
  */
 class ElfCache : public ElfCacheBase {
  public:
-  explicit ElfCache(size_t capacity);
-
   std::shared_ptr<ElfFile> getFile(StringPiece path) override;
 
  private:
   std::mutex mutex_;
 
-  typedef boost::intrusive::list_member_hook<> LruLink;
-
   struct Entry {
     std::string path;
     ElfFile file;
-    LruLink lruLink;
   };
 
   static std::shared_ptr<ElfFile> filePtr(const std::shared_ptr<Entry>& e);
 
-  size_t capacity_;
   std::unordered_map<StringPiece, std::shared_ptr<Entry>, Hash> files_;
-
-  typedef boost::intrusive::list<
-      Entry,
-      boost::intrusive::member_hook<Entry, LruLink, &Entry::lruLink>,
-      boost::intrusive::constant_time_size<false>>
-      LruList;
-  LruList lruList_;
 };
 } // namespace symbolizer
 } // namespace folly

folly/experimental/symbolizer/Symbolizer.cpp

@@ -55,8 +55,7 @@ namespace symbolizer {
 namespace {
 
 ElfCache* defaultElfCache() {
-  static constexpr size_t defaultCapacity = 500;
-  static auto cache = new ElfCache(defaultCapacity);
+  static auto cache = new ElfCache();
   return cache;
 }
 
@@ -512,17 +511,9 @@ void SafeStackTracePrinter::printStackTrace(bool symbolize) {
 
 FastStackTracePrinter::FastStackTracePrinter(
     std::unique_ptr<SymbolizePrinter> printer,
-    size_t elfCacheSize,
     size_t symbolCacheSize)
-    : elfCache_(
-          elfCacheSize == 0
-              ? nullptr
-              : new ElfCache{std::max(countLoadedElfFiles(), elfCacheSize)}),
-      printer_(std::move(printer)),
-      symbolizer_(
-          elfCache_ ? elfCache_.get() : defaultElfCache(),
-          LocationInfoMode::FULL,
-          symbolCacheSize) {}
+    : printer_(std::move(printer)),
+      symbolizer_(defaultElfCache(), LocationInfoMode::FULL, symbolCacheSize) {}
 
 FastStackTracePrinter::~FastStackTracePrinter() = default;
 

folly/experimental/symbolizer/Symbolizer.h

@@ -390,7 +390,6 @@ class FastStackTracePrinter {
 
   explicit FastStackTracePrinter(
       std::unique_ptr<SymbolizePrinter> printer,
-      size_t elfCacheSize = 0, // 0 means "use the default elf cache instance."
       size_t symbolCacheSize = kDefaultSymbolCacheSize);
 
   ~FastStackTracePrinter();
@@ -406,7 +405,6 @@ class FastStackTracePrinter {
  private:
   static constexpr size_t kMaxStackTraceDepth = 100;
 
-  const std::unique_ptr<ElfCache> elfCache_;
   const std::unique_ptr<SymbolizePrinter> printer_;
   Symbolizer symbolizer_;
 };

folly/experimental/symbolizer/test/SymbolizerTest.cpp

@@ -89,10 +89,9 @@ void runElfCacheTest(Symbolizer& symbolizer) {
   }
 }
 
-TEST_F(ElfCacheTest, TinyElfCache) {
-  ElfCache cache(1);
+TEST_F(ElfCacheTest, ElfCache) {
+  ElfCache cache;
   Symbolizer symbolizer(&cache);
-  // Run twice, in case the wrong stuff gets evicted?
   for (size_t i = 0; i < 2; ++i) {
     runElfCacheTest(symbolizer);
   }