Maintain a mapping from funcId -> Func*

This diff adds AtomicVector (see comment in atomic_vector.h
for what it is) and uses it to maintain a mapping from funcIds to the
corresponding Func*. This is then used in sktrace instead of
curUnit(), since curUnit() isn't guaranteed to be the Unit the SrcKey
in question came from.

hphp/runtime/server/admin_request_handler.cpp

@@ -590,6 +590,7 @@ bool AdminRequestHandler::handleCheckRequest(const std::string &cmd,
     appendStat("tc-stubsize", tx->getStubSize());
     appendStat("targetcache", tx->getTargetCacheSize());
     appendStat("units", Eval::FileRepository::getLoadedFiles());
+    appendStat("Funcs", Func::nextFuncId());
     out << "}" << endl;
     transport->sendString(out.str());
     return true;

hphp/runtime/vm/func.cpp

@@ -19,6 +19,7 @@
 #include <boost/scoped_ptr.hpp>
 
 #include "hphp/runtime/base/base_includes.h"
+#include "hphp/util/atomic_vector.h"
 #include "hphp/util/util.h"
 #include "hphp/util/trace.h"
 #include "hphp/util/debug.h"
@@ -84,17 +85,32 @@ void Func::parametersCompat(const PreClass* preClass, const Func* imeth) const {
   }
 }
 
-static FuncId s_nextFuncId = 0;
+static std::atomic<FuncId> s_nextFuncId(0);
 
-void Func::setFuncId(FuncId id) {
-  assert(m_funcId == InvalidFuncId);
-  assert(id != InvalidFuncId);
-  m_funcId = id;
-}
+// This size hint will create a ~2MB vector and is rarely hit in
+// practice. Note that this is just a hint and exceeding it won't
+// affect correctness.
+constexpr size_t kFuncVecSizeHint = 250000;
+static AtomicVector<const Func*> s_funcVec(kFuncVecSizeHint, nullptr);
 
 void Func::setNewFuncId() {
   assert(m_funcId == InvalidFuncId);
-  m_funcId = static_cast<FuncId>(__sync_fetch_and_add(&s_nextFuncId, 1));
+  m_funcId = s_nextFuncId.fetch_add(1, std::memory_order_relaxed);
+
+  s_funcVec.ensureSize(m_funcId + 1);
+  DEBUG_ONLY auto oldVal = s_funcVec.exchange(m_funcId, this);
+  assert(oldVal == nullptr);
+}
+
+FuncId Func::nextFuncId() {
+  return s_nextFuncId.load(std::memory_order_relaxed);
+}
+
+const Func* Func::fromFuncId(FuncId id) {
+  assert(id < s_nextFuncId);
+  auto func = s_funcVec.get(id);
+  func->validate();
+  return func;
 }
 
 void Func::setFullName() {
@@ -227,6 +243,10 @@ Func::~Func() {
   if (m_fullName != nullptr && m_maybeIntercepted != -1) {
     unregister_intercept_flag(fullNameRef(), &m_maybeIntercepted);
   }
+  if (m_funcId != InvalidFuncId) {
+    DEBUG_ONLY auto oldVal = s_funcVec.exchange(m_funcId, nullptr);
+    assert(oldVal == this);
+  }
 #ifdef DEBUG
   validate();
   m_magic = ~m_magic;
@@ -1172,4 +1192,4 @@ void FuncRepoProxy::GetFuncsStmt
   txn.commit();
 }
 
- } // HPHP::VM
+} // HPHP::VM

hphp/runtime/vm/func.h

@@ -159,10 +159,12 @@ struct Func {
 
   FuncId getFuncId() const {
     assert(m_funcId != InvalidFuncId);
+    assert(fromFuncId(m_funcId) == this);
     return m_funcId;
   }
-  void setFuncId(FuncId id);
   void setNewFuncId();
+  static FuncId nextFuncId();
+  static const Func* fromFuncId(FuncId id);
 
   void rename(const StringData* name);
   int numSlotsInFrame() const;

hphp/runtime/vm/jit/translator.cpp

@@ -181,25 +181,6 @@ std::string Tracelet::toString() const {
   return out.str();
 }
 
-void sktrace(SrcKey sk, const char *fmt, ...) {
-  if (!Trace::enabled) {
-    return;
-  }
-  // We don't want to print string literals, so don't pass the unit
-  string s = instrToString((Op*)curUnit()->at(sk.offset()));
-  const char *filepath = "*anonFile*";
-  if (curUnit()->filepath()->data() &&
-      strlen(curUnit()->filepath()->data()) > 0)
-    filepath = curUnit()->filepath()->data();
-  Trace::trace("%s:%llx %6d: %20s ",
-               filepath, (unsigned long long)sk.getFuncId(),
-               sk.offset(), s.c_str());
-  va_list a;
-  va_start(a, fmt);
-  Trace::vtrace(fmt, a);
-  va_end(a);
-}
-
 SrcKey Tracelet::nextSk() const {
   return m_instrStream.last->source.advanced(curUnit());
 }

hphp/runtime/vm/jit/translator.h

@@ -77,10 +77,6 @@ enum class VMRegState {
 };
 extern __thread VMRegState tl_regState;
 
-void sktrace(SrcKey sk, const char *fmt, ...) ATTRIBUTE_PRINTF(2,3);
-#define SKTRACE(level, sk, ...) \
-  ONTRACE(level, sktrace(sk, __VA_ARGS__))
-
 struct NormalizedInstruction;
 
 // A DynLocation is a Location-in-execution: a location, along with

hphp/runtime/vm/srckey.cpp

@@ -0,0 +1,43 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2013 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+
+#include "hphp/runtime/vm/srckey.h"
+
+namespace HPHP {
+
+void sktrace(SrcKey sk, const char *fmt, ...) {
+  if (!Trace::enabled) {
+    return;
+  }
+  // We don't want to print string literals, so don't pass the unit
+  auto func = sk.func();
+  auto unit = sk.unit();
+  string s = instrToString((Op*)unit->at(sk.offset()));
+  const char *filepath = "*anonFile*";
+  if (unit->filepath()->data() && strlen(unit->filepath()->data()) > 0) {
+    filepath = unit->filepath()->data();
+  }
+  Trace::trace("%s:%d in %s(id 0x%llx) %6d: %20s ",
+               filepath, unit->getLineNumber(sk.offset()),
+               func->isPseudoMain() ? "pseudoMain" : func->fullName()->data(),
+               (unsigned long long)sk.getFuncId(), sk.offset(), s.c_str());
+  va_list a;
+  va_start(a, fmt);
+  Trace::vtrace(fmt, a);
+  va_end(a);
+}
+
+}

hphp/runtime/vm/srckey.h

@@ -73,6 +73,14 @@ struct SrcKey : private boost::totally_ordered<SrcKey> {
     return m_funcId;
   }
 
+  const Func* func() const {
+    return Func::fromFuncId(m_funcId);
+  }
+
+  const Unit* unit() const {
+    return func()->unit();
+  }
+
   void setOffset(Offset o) {
     m_offset = o;
   }
@@ -129,6 +137,10 @@ inline std::string show(SrcKey sk) {
   return folly::format("{}@{}", sk.getFuncId(), sk.offset()).str();
 }
 
+void sktrace(SrcKey sk, const char *fmt, ...) ATTRIBUTE_PRINTF(2,3);
+#define SKTRACE(level, sk, ...) \
+  ONTRACE(level, sktrace(sk, __VA_ARGS__))
+
 }
 
 #endif

hphp/util/atomic_vector.h

@@ -0,0 +1,149 @@
+/*
+   +----------------------------------------------------------------------+
+   | HipHop for PHP                                                       |
+   +----------------------------------------------------------------------+
+   | Copyright (c) 2010-2013 Facebook, Inc. (http://www.facebook.com)     |
+   +----------------------------------------------------------------------+
+   | This source file is subject to version 3.01 of the PHP license,      |
+   | that is bundled with this package in the file LICENSE, and is        |
+   | available through the world-wide-web at the following url:           |
+   | http://www.php.net/license/3_01.txt                                  |
+   | If you did not receive a copy of the PHP license and are unable to   |
+   | obtain it through the world-wide-web, please send a note to          |
+   | license@php.net so we can mail you a copy immediately.               |
+   +----------------------------------------------------------------------+
+*/
+
+#ifndef incl_HPHP_ATOMIC_VECTOR_H
+#define incl_HPHP_ATOMIC_VECTOR_H
+
+#include <atomic>
+
+#include "folly/String.h"
+
+#include "hphp/util/util.h"
+#include "hphp/util/trace.h"
+
+namespace HPHP {
+
+/*
+ * AtomicVector is a simple vector intended for use by many concurrent readers
+ * and writers. The size given to the constructor determines how many elements
+ * the AtomicVector will initially hold, and each one will be initialized to
+ * the given default value. Elements may be retrieved and exchanged with any
+ * valid index by many readers and writers concurrently, though the operations
+ * may be very slow if std::atomic<Value>::is_lock_free() == false.
+ *
+ * The only way to increase the size of an AtomicVector is with the ensureSize
+ * method. It does not reallocate the internal storage to grow; it allocates a
+ * new AtomicVector and chains to that for increased capacity. This means that
+ * if the initial size is too low, reading and modifying elements at high
+ * indexes will be increasingly slower as the chain of AtomicVectors is walked
+ * to find the right element.
+ *
+ * An AtomicVector cannot shrink, and will only reclaim memory when destructed.
+ */
+
+template<typename Value>
+class AtomicVector {
+ public:
+  AtomicVector(size_t size, const Value& def);
+  ~AtomicVector();
+
+  void ensureSize(size_t size);
+  Value exchange(size_t i, const Value& val);
+
+  Value get(size_t i) const;
+
+ private:
+  static std::string typeName();
+
+  size_t m_size;
+  std::atomic<AtomicVector*> m_next;
+  const Value m_default;
+  std::unique_ptr<std::atomic<Value>[]> m_vals;
+  TRACE_SET_MOD(atomicvector);
+};
+
+template<typename Value>
+std::string AtomicVector<Value>::typeName() {
+  auto name = folly::demangle(typeid(Value));
+  return folly::format("AtomicVector<{}>", name).str();
+}
+
+template<typename Value>
+AtomicVector<Value>::AtomicVector(size_t size, const Value& def)
+  : m_size(size)
+  , m_next(nullptr)
+  , m_default(def)
+  , m_vals(new std::atomic<Value>[size])
+{
+  FTRACE(1, "{} {} constructing with size {}, default {}\n",
+         typeName(), this, size, def);
+  assert(size > 0 && "size must be nonzero");
+
+  for (size_t i = 0; i < size; ++i) {
+    m_vals[i] = def;
+  }
+}
+
+template<typename Value>
+AtomicVector<Value>::~AtomicVector() {
+  FTRACE(1, "{} {} destructing\n",
+         typeName(), this);
+
+  delete m_next.load(std::memory_order_relaxed);
+}
+
+template<typename Value>
+void AtomicVector<Value>::ensureSize(size_t size) {
+  FTRACE(2, "{}::ensureSize({}), m_size = {}\n",
+         typeName(), size, m_size);
+  if (m_size >= size) return;
+
+  if (!m_next.load(std::memory_order_acquire)) {
+    auto next = folly::make_unique<AtomicVector>(m_size * 2, m_default);
+    AtomicVector* expected = nullptr;
+    FTRACE(2, "Attempting to use {}...", next.get());
+    if (!m_next.compare_exchange_strong(expected, next.get(),
+                                        std::memory_order_acq_rel)) {
+      FTRACE(2, "lost race to {}\n", expected);
+    } else {
+      FTRACE(2, "success\n");
+      next.reset();
+    }
+  }
+
+  m_next.load(std::memory_order_acquire)->ensureSize(size - m_size);
+}
+
+template<typename Value>
+Value AtomicVector<Value>::exchange(size_t i, const Value& val) {
+  FTRACE(3, "{}::exchange({}, {}), m_size = {}\n",
+         typeName(), i, val, m_size);
+  if (i < m_size) {
+    auto oldVal = m_vals[i].exchange(val, std::memory_order_acq_rel);
+    FTRACE(3, "{}::exchange returning {}\n", typeName(), oldVal);
+    return oldVal;
+  }
+
+  assert(m_next);
+  return m_next.load(std::memory_order_acquire)->exchange(i - m_size, val);
+}
+
+template<typename Value>
+Value AtomicVector<Value>::get(size_t i) const {
+  FTRACE(4, "{}::get({}), m_size = {}\n", typeName(), i, m_size);
+  if (i < m_size) {
+    auto val = m_vals[i].load(std::memory_order_acquire);
+    FTRACE(5, "{}::get returning {}\n", typeName(), m_vals[i].load());
+    return val;
+  }
+
+  assert(m_next);
+  return m_next.load(std::memory_order_acquire)->get(i - m_size);
+}
+
+}
+
+#endif

hphp/util/trace.h

@@ -96,6 +96,7 @@ namespace Trace {
       TM(statgroups)  \
       TM(minstr)      \
       TM(region)      \
+      TM(atomicvector)\
       /* Stress categories, to exercise rare paths */ \
       TM(stress_txInterpPct)    \
       TM(stress_txInterpSeed)   \