new opcode CreateCl

This is the bottleneck for closure perf. Instead of all the code in the prologue, it turned out the expensive thing about closures was constructing them. This bypasses all the roundabout copying involved with __construct and makes a new opcode to do the same thing.

hphp/compiler/analysis/emitter.cpp

@@ -1773,7 +1773,6 @@ void EmitterVisitor::visit(FileScopePtr file) {
   emitPostponedPinits();
   emitPostponedSinits();
   emitPostponedCinits();
-  emitPostponedClosureCtors();
   Peephole peephole(m_ue, m_metaInfo);
   m_metaInfo.setForUnit(m_ue);
 }
@@ -3801,16 +3800,11 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) {
         // We're still at the closure definition site. Emit code to instantiate
         // the new anonymous class, with the use variables as arguments.
         ExpressionListPtr valuesList(ce->getClosureValues());
-        Offset fpiStart = m_ue.bcPos();
-        e.FPushCtorD(useCount, className);
-        {
-          FPIRegionRecorder fpi(this, m_ue, m_evalStack, fpiStart);
-          for (int i = 0; i < useCount; ++i) {
-            emitFuncCallArg(e, (*valuesList)[i], i);
-          }
+        for (int i = 0; i < useCount; ++i) {
+          emitBuiltinCallArg(e, (*valuesList)[i], i, useVars[i].second);
         }
-        e.FCall(useCount);
-        emitPop(e);
+        e.CreateCl(useCount, className);
+
         // From here on out, we're just building metadata for the closure.
 
         // Instance variables.
@@ -3821,15 +3815,6 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) {
                            &uninit, KindOfInvalid);
         }
 
-        // The constructor. This is entirely generated; all it does is stash its
-        // arguments in the object's instance variables.
-        static const StringData* ctorName =
-          StringData::GetStaticString("__construct");
-        FuncEmitter* ctor = m_ue.newMethodEmitter(ctorName, pce);
-        pce->addMethod(ctor);
-        m_postponedClosureCtors.push_back(
-          PostponedClosureCtor(useVars, ce, ctor));
-
         // The __invoke method. This is the body of the closure, preceded by
         // code that pulls the object's instance variables into locals.
         static const StringData* invokeName =
@@ -5638,74 +5623,6 @@ void EmitterVisitor::emitPostponedCinits() {
   }
 }
 
-void EmitterVisitor::emitPostponedClosureCtors() {
-  while (!m_postponedClosureCtors.empty()) {
-    PostponedClosureCtor& ctor = m_postponedClosureCtors.front();
-    ClosureUseVarVec& useVars = ctor.m_useVars;
-    FuncEmitter* fe = ctor.m_fe;
-    const Location* sLoc = ctor.m_expr->getLocation().get();
-    fe->init(sLoc->line0, sLoc->line1, m_ue.bcPos(), AttrPublic, false, nullptr);
-
-    unsigned n = useVars.size();
-    Emitter e(ctor.m_expr, m_ue, *this);
-    FuncFinisher ff(this, e, fe);
-    if (n > 0) {
-      for (unsigned i = 0; i < n; ++i) {
-        // To ensure that we get a new local for every use var, we call
-        // appendParam with an artificial uniquified name. Because there's no
-        // user code here, the fact that the variable has a made-up name in the
-        // metadata doesn't matter.
-        std::ostringstream num;
-        num << i;
-        FuncEmitter::ParamInfo pi;
-        pi.setRef(useVars[i].second);
-        fe->appendParam(StringData::GetStaticString(num.str()), pi);
-        if (i) {
-          m_metaInfo.add(m_ue.bcPos(), Unit::MetaInfo::GuardedThis,
-                      false, 0, 0);
-        }
-        e.CheckThis();
-        m_evalStack.push(StackSym::H);
-        m_evalStack.push(StackSym::T);
-        m_evalStack.setString(useVars[i].first);
-        markProp(e);
-        emitVirtualLocal(i);
-        if (useVars[i].second) {
-          emitVGet(e);
-          emitBind(e);
-        } else {
-          emitCGet(e);
-          emitSet(e);
-        }
-        emitPop(e);
-      }
-    }
-
-    // call parent::__construct()
-    static StringData* s___construct = StringData::GetStaticString("__construct");
-    e.String(s___construct);
-    static StringData* s_Closure = StringData::GetStaticString("Closure");
-    e.String(s_Closure);
-    e.AGetC();
-    Offset fpiStart = m_ue.bcPos();
-    e.FPushClsMethodF(0);
-    {
-      FPIRegionRecorder fpi(this, m_ue, m_evalStack, fpiStart);
-    }
-    e.FCall(0);
-    e.PopR();
-
-    e.Null();
-    if (n > 0) {
-      m_metaInfo.add(m_ue.bcPos(), Unit::MetaInfo::GuardedThis,
-                  false, 0, 0);
-    }
-    e.RetC();
-
-    m_postponedClosureCtors.pop_front();
-  }
-}
-
 void EmitterVisitor::emitVirtualLocal(int localId,
                                       DataType dt /* = KindOfUnknown */) {
   prepareEvalStack();

hphp/doc/bytecode.specification

@@ -3617,6 +3617,12 @@ AKExists    [C C] -> [C:Bool]
   or object, and raises a warning if $2 is not a string, integer, or
   null.
 
+CreateCl <num args> <class name>  [F..F]  ->  [C]
+
+  Creates an instance of <class name> and pushes it on the stack.
+  <class name> must be a subclass of "Closure".
+
+
 14. Continuation creation and execution
 ---------------------------------------
 

hphp/doc/ir.specification

@@ -1200,7 +1200,14 @@ FreeSpill S0:ConstInt
   Deallocates S0 slots of spill space on the stack.
 
 
-16. Continuations
+16. Continuations & Closures
+
+CreateCl S0:Int S1:Str S2:StkPtr S3:StkPtr
+
+  Creates an object of class S1 (which must be a subclass of Closure),
+  copies S0 number of parameters from S2 into the properties on the object,
+  saves the $this and Func* from ActRec S3 into the object,
+  and returns the object.
 
 CreateCont
 FillContLocals

hphp/runtime/ext/ext_closure.cpp

@@ -32,16 +32,14 @@ c_Closure::~c_Closure() {
 }
 
 void c_Closure::t___construct() {
-  VM::ActRec* ar;
-  {
-    // like calling CallerFrame twice
-    VM::Transl::VMRegAnchor _;
-    VMExecutionContext* context = g_vmContext;
-    VM::ActRec* me = context->getFP();
-    VM::ActRec* childClosure = context->getPrevVMState(me);
-    ar = context->getPrevVMState(childClosure);
-  }
-  
+  raise_error("Can't create a Closure directly");
+}
+
+/**
+ * sp points to the last use variable on the stack.
+ * returns the closure so that translator-x64 can just return "rax".
+ */
+c_Closure* c_Closure::init(int numArgs, VM::ActRec* ar, TypedValue* sp) {
   static StringData* invokeName = StringData::GetStaticString("__invoke");
   VM::Func* invokeFunc = getVMClass()->lookupMethod(invokeName);
 
@@ -60,6 +58,16 @@ void c_Closure::t___construct() {
   // Change my __invoke's m_cls to be the same as my creator's
   VM::Class* scope = ar->m_func->cls();
   m_func = invokeFunc->cloneAndSetClass(scope);
+
+  // copy the props to instance variables
+  assert(m_cls->numDeclProperties() == numArgs);
+  TypedValue* beforeCurUseVar = sp + numArgs;
+  TypedValue* curProperty = propVec();
+  for (int i = 0; i < numArgs; i++) {
+    // teleport the references in here so we don't incref
+    *curProperty++ = *--beforeCurUseVar;
+  }
+  return this;
 }
 
 ObjectData* c_Closure::clone() {

hphp/runtime/ext/ext_closure.h

@@ -44,6 +44,8 @@ class c_Closure : public ExtObjectData {
   public: c_Closure *create();
 public:
   public: ObjectData* clone();
+
+  c_Closure* init(int numArgs, VM::ActRec* ar, TypedValue* sp);
   ObjectData* getThisOrClass() { return m_thisOrClass; }
   const VM::Func* getInvokeFunc() { return m_func; }
   HphpArray* getStaticLocals();

hphp/runtime/vm/bytecode.cpp

@@ -6683,6 +6683,18 @@ inline void OPTBLD_INLINE VMExecutionContext::iopParent(PC& pc) {
   m_stack.pushClass(parent);
 }
 
+inline void OPTBLD_INLINE VMExecutionContext::iopCreateCl(PC& pc) {
+  NEXT();
+  DECODE_IVA(numArgs);
+  DECODE_LITSTR(clsName);
+  Class* cls = Unit::loadClass(clsName);
+  c_Closure* cl = static_cast<c_Closure*>(newInstance(cls));
+  c_Closure* cl2 = cl->init(numArgs, m_fp, m_stack.top());
+  m_stack.ndiscard(numArgs);
+  assert(cl == cl2);
+  m_stack.pushObject(cl2);
+}
+
 template<bool isMethod>
 c_Continuation*
 VMExecutionContext::createContinuation(ActRec* fp,

hphp/runtime/vm/hhbc.h

@@ -552,6 +552,7 @@ enum SetOpOp {
   O(Parent,          NA,               NOV,             ONE(AV),    NF) \
   O(LateBoundCls,    NA,               NOV,             ONE(AV),    NF) \
   O(NativeImpl,      NA,               NOV,             NOV,        CF_TF) \
+  O(CreateCl,        TWO(IVA,SA),      FMANY,           ONE(CV),    NF) \
   O(CreateCont,      TWO(IVA,SA),      NOV,             ONE(CV),    NF) \
   O(ContEnter,       NA,               NOV,             NOV,        CF) \
   O(ContExit,        NA,               NOV,             NOV,        CF) \

hphp/runtime/vm/translator/hopt/codegen.cpp

@@ -28,6 +28,7 @@
 #include "runtime/base/runtime_option.h"
 #include "runtime/base/string_data.h"
 #include "runtime/base/types.h"
+#include "runtime/ext/ext_closure.h"
 #include "runtime/ext/ext_continuation.h"
 #include "runtime/vm/bytecode.h"
 #include "runtime/vm/runtime.h"
@@ -3260,7 +3261,7 @@ void CodeGenerator::emitSpillActRec(SSATmp* sp,
 }
 
 HOT_FUNC_VM ActRec*
-cgNewInstanceHelper(Class* cls,
+irNewInstanceHelper(Class* cls,
                     int numArgs,
                     Cell* sp,
                     ActRec* prevAr) {
@@ -3275,7 +3276,7 @@ cgNewInstanceHelper(Class* cls,
 }
 
 HOT_FUNC_VM static ActRec*
-cgNewInstanceHelperCached(CacheHandle cacheHandle,
+irNewInstanceHelperCached(CacheHandle cacheHandle,
                           const StringData* clsName,
                           int numArgs,
                           Cell* sp,
@@ -3283,11 +3284,13 @@ cgNewInstanceHelperCached(CacheHandle cacheHandle,
   Cell* obj = sp - 1; // this is where the newly allocated object will go
   ActRec* ar = (ActRec*)(uintptr_t(obj) - sizeof(ActRec));
 
-  Instance* newObj = newInstanceHelperCached((Class**)handleToPtr(cacheHandle),
-                                             clsName,
-                                             numArgs,
-                                             ar,
-                                             prevAr);
+  Instance* newObj = newInstanceHelperCached(
+    static_cast<Class**>(handleToPtr(cacheHandle)),
+    clsName,
+    numArgs,
+    ar,
+    prevAr
+  );
   // store obj into the stack
   obj->m_data.pobj = newObj;
   obj->m_type = KindOfObject;
@@ -3306,15 +3309,25 @@ static inline ALWAYS_INLINE Class* getKnownClass(Class** classCache,
   return cls;
 }
 
+static Instance*
+newInstanceHelperNoCtorCachedInstance(Class** classCache,
+                                      const StringData* clsName) {
+  Class* cls = getKnownClass(classCache, clsName);
+  Instance* newObj = newInstance(cls);
+  newObj->incRefCount();
+  return newObj;
+}
+
 static Cell*
 newInstanceHelperNoCtorCached(CacheHandle cacheHandle,
                                 const StringData* clsName,
                                 Cell* sp) {
   Cell* obj = sp - 1; // this is where the newly allocated object will go
 
-  Class* cls = getKnownClass((Class**)handleToPtr(cacheHandle), clsName);
-  Instance* newObj = newInstance(cls);
-  newObj->incRefCount();
+  Instance* newObj = newInstanceHelperNoCtorCachedInstance(
+    (Class**)handleToPtr(cacheHandle),
+    clsName
+  );
 
   // store obj into the stack
   obj->m_data.pobj = newObj;
@@ -3334,7 +3347,7 @@ void CodeGenerator::cgNewObjCached(IRInstruction* inst) {
   CacheHandle ch = allocKnownClass(classNameString);
 
   ActRec* (*helper)(CacheHandle, const StringData*, int, Cell*, ActRec*)
-    = cgNewInstanceHelperCached;
+    = irNewInstanceHelperCached;
   ArgGroup args;
   args.imm(ch)
     .ssa(clsName)
@@ -3366,6 +3379,43 @@ void CodeGenerator::cgNewObjNoCtorCached(IRInstruction* inst) {
                args);
 }
 
+void CodeGenerator::cgCreateCl(IRInstruction* inst) {
+  SSATmp* dst   = inst->getDst();
+  SSATmp* numParams = inst->getSrc(0);
+  SSATmp* clsName = inst->getSrc(1);
+  SSATmp* sp    = inst->getSrc(2);
+  SSATmp* fp    = inst->getSrc(3);
+
+  const StringData* classNameString = clsName->getValStr();
+  CacheHandle ch = allocKnownClass(classNameString);
+  Class** classCache = static_cast<Class**>(handleToPtr(ch));
+
+  Instance* (*helper)(Class** classCache, const StringData* clsName) =
+    newInstanceHelperNoCtorCachedInstance;
+  cgCallHelper(
+    m_as,
+    (TCA)helper,
+    rScratch,
+    kSyncPoint,
+    ArgGroup()
+      .immPtr<Class*>(classCache)
+      .ssa(clsName)
+  );
+
+  cgCallHelper(
+    m_as,
+    Transl::Call(getMethodPtr(&c_Closure::init)),
+    dst->getReg(),
+    kSyncPoint,
+    ArgGroup()
+      .reg(rScratch)
+      .ssa(numParams)
+      .ssa(fp)
+      .ssa(sp)
+  );
+}
+
+
 void CodeGenerator::cgCall(IRInstruction* inst) {
   SSATmp* actRec         = inst->getSrc(0);
   SSATmp* returnBcOffset = inst->getSrc(1);

hphp/runtime/vm/translator/hopt/codegen.h

@@ -458,7 +458,7 @@ private:
 };
 
 using namespace HPHP::VM::Transl::TargetCache;
-ActRec* cgNewInstanceHelper(Class* cls,
+ActRec* irNewInstanceHelper(Class* cls,
                             int numArgs,
                             Cell* sp,
                             ActRec* prevAr);

hphp/runtime/vm/translator/hopt/hhbctranslator.cpp

@@ -16,6 +16,7 @@
 #include "runtime/vm/translator/hopt/hhbctranslator.h"
 
 #include "util/trace.h"
+#include "runtime/ext/ext_closure.h"
 #include "runtime/ext/ext_continuation.h"
 #include "runtime/vm/translator/translator-runtime.h"
 #include "runtime/vm/translator/translator-x64.h"
@@ -1294,6 +1295,21 @@ void HhbcTranslator::emitFPushCtorD(int32_t numParams, int32_t classNameStrId) {
   m_tb->genNewObjCached(numParams, className);
 }
 
+void HhbcTranslator::emitCreateCl(int32_t numParams, int32_t funNameStrId) {
+  const StringData* clsName = lookupStringId(funNameStrId);
+  TRACE(3, "%u: CreateCl %d %s\n", m_bcOff, numParams, clsName->data());
+  spillStack();
+  SSATmp* closure = m_tb->gen(
+    CreateCl,
+    m_tb->genDefConst(numParams),
+    m_tb->genDefConst(clsName),
+    m_tb->getSp(),
+    m_tb->getFp()
+  );
+  discard(numParams);
+  push(closure);
+}
+
 void HhbcTranslator::emitFPushFuncD(int32_t numParams, int32_t funcId) {
   TRACE(3, "%u: FPushFuncD %d %d\n", m_bcOff, numParams, funcId);
   const NamedEntityPair& nep = lookupNamedEntityPairId(funcId);

hphp/runtime/vm/translator/hopt/hhbctranslator.h

@@ -232,6 +232,7 @@ struct HhbcTranslator {
                            const StringData* methName);
   void emitFPushCtorD(int32_t numParams, int32_t classNameStrId);
   void emitFPushCtor(int32_t numParams);
+  void emitCreateCl(int32_t numParams, int32_t classNameStrId);
   void emitFCallArray();
   void emitFCall(uint32_t numParams,
                   Offset returnBcOffset,

hphp/runtime/vm/translator/hopt/ir.h

@@ -301,6 +301,10 @@ O(NewObjCached,              D(StkPtr), C(Int)                                \
                                           S(StkPtr),     E|Mem|N|Refs|PRc|Er) \
 O(NewObjNoCtorCached,        D(StkPtr), S(Str)                                \
                                           S(StkPtr),     E|Mem|N|Refs|PRc|Er) \
+O(CreateCl,                     D(Obj), C(Int)                                \
+                                          S(Str)                              \
+                                          S(StkPtr)                           \
+                                          S(StkPtr),                   Mem|N) \
 O(NewArray,                     D(Arr), C(Int),                  E|Mem|N|PRc) \
 O(NewTuple,                     D(Arr), C(Int) S(StkPtr),    E|Mem|N|PRc|CRc) \
 O(LdRaw,                        DParam, SUnk,                             NF) \

hphp/runtime/vm/translator/hopt/irtranslator.cpp

@@ -1010,6 +1010,11 @@ void TranslatorX64::irTranslateFPushCtorD(const Tracelet& t,
   HHIR_EMIT(FPushCtorD, (i.imm[0].u_IVA), (i.imm[1].u_SA));
 }
 
+void TranslatorX64::irTranslateCreateCl(const Tracelet& t,
+                                        const NormalizedInstruction& i) {
+  HHIR_EMIT(CreateCl, (i.imm[0].u_IVA), (i.imm[1].u_SA));
+}
+
 // static void fatalNullThis() { raise_error(Strings::FATAL_NULL_THIS); }
 
 void

hphp/runtime/vm/translator/hopt/nativecalls.cpp

@@ -75,7 +75,7 @@ static CallMap s_callMap({
     {NewArray,           (TCA)new_array, DSSA, SNone, {{SSA, 0}}},
     {NewTuple,           (TCA)new_tuple, DSSA, SNone,
                            {{SSA, 0}, {SSA, 1}}},
-    {NewObj,             (TCA)cgNewInstanceHelper, DSSA, SSync,
+    {NewObj,             (TCA)irNewInstanceHelper, DSSA, SSync,
                            {{SSA, 0}, {SSA, 1}, {SSA, 2}, {SSA, 3}}},
     {PrintStr,           (TCA)print_string, DNone, SNone, {{SSA, 0}}},
     {PrintInt,           (TCA)print_int, DNone, SNone, {{SSA, 0}}},

hphp/runtime/vm/translator/translator-x64.cpp

@@ -9338,6 +9338,33 @@ TranslatorX64::emitFPushCtorDFast(const NormalizedInstruction& i,
   emitVStackStore(a, i, rax, arOff + AROFF(m_this));
 }
 
+void
+TranslatorX64::translateCreateCl(const Tracelet& t,
+                                 const NormalizedInstruction& i) {
+  int getArgs = i.imm[0].u_IVA;
+  const StringData* clsName = curUnit()->lookupLitstrId(i.imm[1].u_SA);
+
+  LazyScratchReg clsCache(m_regMap);
+
+  TargetCache::CacheHandle classCh = TargetCache::allocKnownClass(clsName);
+  clsCache.alloc();
+  a.    lea_reg64_disp_reg64(rVmTl, classCh, r(clsCache));
+  EMIT_RCALL(a, i,
+             newInstanceHelperNoCtorCached,
+             R(clsCache),
+             IMM(uintptr_t(clsName)));
+
+  EMIT_RCALL(a, i,
+             getMethodPtr(&c_Closure::init),
+             R(rax),
+             IMM(getArgs),
+             R(rVmFp),
+             RPLUS(rVmSp, vstackOffset(i, 0)));
+
+  m_regMap.bind(rax, i.outStack->location, KindOfObject, RegInfo::DIRTY);
+}
+
+
 static void fatalNullThis() {
   raise_error(Strings::FATAL_NULL_THIS);
 }
@@ -12379,6 +12406,7 @@ bool TranslatorX64::dumpTCData() {
   SUPPORTED_OP(ContCurrent) \
   SUPPORTED_OP(FPushCtor) \
   SUPPORTED_OP(FPushCtorD) \
+  SUPPORTED_OP(CreateCl) \
   SUPPORTED_OP(StaticLocInit) \
   /*
    * Always-interp instructions,

hphp/runtime/vm/translator/translator-x64.h

@@ -615,6 +615,7 @@ MINSTRS
   CASE(InterfaceExists) \
   CASE(TraitExists) \
   CASE(Dup) \
+  CASE(CreateCl) \
   CASE(CreateCont) \
   CASE(ContEnter) \
   CASE(ContExit) \

hphp/runtime/vm/translator/translator.cpp

@@ -1209,6 +1209,7 @@ static const struct {
   { OpParent,      {None,             Stack1,       OutClassRef,       1 }},
   { OpLateBoundCls,{None,             Stack1,       OutClassRef,       1 }},
   { OpNativeImpl,  {None,             None,         OutNone,           0 }},
+  { OpCreateCl,    {BStackN,          Stack1,       OutObject,         1 }},
 
   /*** 14. Continuation instructions ***/
 
@@ -1252,15 +1253,16 @@ int getStackDelta(const NormalizedInstruction& ni) {
   int hiddenStackInputs = 0;
   initInstrInfo();
   Opcode op = ni.op();
-  if (op == OpFCall) {
-    int numArgs = ni.imm[0].u_IVA;
-    return 1 - numArgs - kNumActRecCells;
-  }
-  if (op == OpFCallBuiltin) {
-    return 1 - ni.imm[0].u_IVA;
-  }
-  if (op == OpNewTuple) {
-    return 1 - ni.imm[0].u_IVA;
+  switch (op) {
+    case OpFCall: {
+      int numArgs = ni.imm[0].u_IVA;
+      return 1 - numArgs - kNumActRecCells;
+    }
+
+    case OpFCallBuiltin:
+    case OpNewTuple:
+    case OpCreateCl:
+      return 1 - ni.imm[0].u_IVA;
   }
   const InstrInfo& info = instrInfo[op];
   if (info.in & MVector) {

hphp/runtime/vm/verifier/check_func.cpp

@@ -605,7 +605,8 @@ const FlavorDesc* FuncChecker::sig(PC pc) {
     return vectorSig(pc, CV);
   case OpFCall:     // ONE(IVA),     FMANY,   ONE(RV)
   case OpFCallArray:// NA,           ONE(FV), ONE(RV)
-  case OpFCallBuiltin: //TWO(IVA, SA) FMANY,   ONE(RV)
+  case OpFCallBuiltin: //TWO(IVA, SA) FMANY,  ONE(RV)
+  case OpCreateCl:  // TWO(IVA,SA),  FMANY,   ONE(CV)
     for (int i = 0, n = instrNumPops(pc); i < n; ++i) {
       m_tmp_sig[i] = FV;
     }