From 39416bcaa5a0d1fdd25c13b37372d9ce9baa3515 Mon Sep 17 00:00:00 2001 From: Guilherme Ottoni Date: Mon, 6 May 2013 18:31:56 -0700 Subject: [PATCH] Use target cache for static properties of persistent classes Accessibility of properties of persistent classes can be checked at JIT time, so use the SProp-based fast path for these cases too. Also, enhance HhbcTranslator to try to emit LdClsPropAddrCached, instead of relying on the Simplifier to transform LdClsPropAddr into LdClsPropAddrCached. Doing this only in the Simplifier results in unecessary ExceptionBarrier before LdClsPropAddrCached. However, there's still value in keeping the optimization in the Simplifier, so that it can leverage opportunities exposed by other optimizations. So factor out the logic to check whether to use the SProp cache. --- .../vm/translator/hopt/hhbctranslator.cpp | 22 ++++- .../vm/translator/hopt/hhbctranslator.h | 2 + hphp/runtime/vm/translator/hopt/ir.cpp | 18 ++++ hphp/runtime/vm/translator/hopt/ir.h | 6 ++ .../runtime/vm/translator/hopt/simplifier.cpp | 82 ++++++++++++------- hphp/runtime/vm/translator/hopt/simplifier.h | 8 ++ hphp/runtime/vm/translator/targetcache.cpp | 6 ++ hphp/runtime/vm/translator/targetcache.h | 1 + hphp/runtime/vm/translator/translator-x64.cpp | 7 -- hphp/runtime/vm/translator/translator-x64.h | 1 - 10 files changed, 113 insertions(+), 40 deletions(-) diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index daa293bde..489ce1369 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -1123,8 +1123,28 @@ SSATmp* HhbcTranslator::getStrName(const StringData* knownName) { return name; } +SSATmp* HhbcTranslator::emitLdClsPropAddrCached(const StringData* propName, + Block* block) { + SSATmp* cls = popA(); + const StringData* clsName = findClassName(cls); + assert(clsName); + + SSATmp* prop = getStrName(propName); + SSATmp* addr = gen(LdClsPropAddrCached, + block, + cls, + prop, + cns(clsName), + cns(getCurClass())); + return addr; +} + SSATmp* HhbcTranslator::emitLdClsPropAddrOrExit(const StringData* propName, Block* block) { + if (canUseSPropCache(m_evalStack.top(), propName, getCurClass())) { + return emitLdClsPropAddrCached(propName, block); + } + if (!block) exceptionBarrier(); SSATmp* clsTmp = popA(); @@ -1522,7 +1542,7 @@ void HhbcTranslator::emitFPushCtorD(int32_t numParams, int32_t classNameStrId) { const Class* cls = Unit::lookupUniqueClass(className); bool uniqueCls = classIsUnique(cls); - bool persistentCls = classIsPersistent(cls); + bool persistentCls = TargetCache::classIsPersistent(cls); bool canInstantiate = canInstantiateClass(cls); bool fastAlloc = !RuntimeOption::EnableObjDestructCall && persistentCls && canInstantiate; diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index 40c8cdb11..245a2d69f 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -533,6 +533,8 @@ private: SSATmp* emitLdClsPropAddr(const StringData* propName) { return emitLdClsPropAddrOrExit(propName, nullptr); } + SSATmp* emitLdClsPropAddrCached(const StringData* propName, + Block* block); SSATmp* getStrName(const StringData* propName = nullptr); SSATmp* emitLdGblAddrDef(const StringData* gblName = nullptr); SSATmp* emitLdGblAddr(const StringData* gblName, Block* block); diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index ec37fef00..586a79616 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -525,6 +525,24 @@ Range IRInstruction::getDsts() const { return Range(m_dst, m_numDsts); } +const StringData* findClassName(SSATmp* cls) { + assert(cls->isA(Type::Cls)); + + if (cls->isConst()) { + return cls->getValClass()->preClass()->name(); + } + // Try to get the class name from a LdCls + IRInstruction* clsInst = cls->inst(); + if (clsInst->op() == LdCls || clsInst->op() == LdClsCached) { + SSATmp* clsName = clsInst->getSrc(0); + assert(clsName->isA(Type::Str)); + if (clsName->isConst()) { + return clsName->getValStr(); + } + } + return nullptr; +} + Opcode negateQueryOp(Opcode opc) { assert(isQueryOp(opc)); diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index d5d2e4565..5d0745cb8 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -1916,6 +1916,12 @@ typedef boost::intrusive::member_hook InstructionList; +/* + * Given an SSATmp of type Cls, try to find the name of the class. + * Returns nullptr if can't find it. + */ +const StringData* findClassName(SSATmp* cls); + /* * Return the output type from a given IRInstruction. * diff --git a/hphp/runtime/vm/translator/hopt/simplifier.cpp b/hphp/runtime/vm/translator/hopt/simplifier.cpp index 4c0bf389b..c70c0ec10 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.cpp +++ b/hphp/runtime/vm/translator/hopt/simplifier.cpp @@ -19,7 +19,7 @@ #include #include -#include +#include "runtime/base/type_conversions.h" #include "runtime/vm/translator/hopt/tracebuilder.h" #include "runtime/vm/runtime.h" @@ -178,6 +178,42 @@ void copyProp(IRInstruction* inst) { } } +/* + * Checks if property propName of class clsTmp, called from context class ctx, + * can be accessed via the static property cache. + * Right now, this returns true for two cases: + * (a) the property is accessed from within the class containing it + * (b) the property belongs to a persistent class and it's accessible from ctx + */ +bool canUseSPropCache(SSATmp* clsTmp, + const StringData* propName, + const Class* ctx) { + if (propName == nullptr) return false; + + const StringData* clsName = findClassName(clsTmp); + if (ctx) { + const StringData* ctxName = ctx->preClass()->name();; + if (clsName && ctxName && clsName->isame(ctxName)) return true; + } + + if (!clsTmp->isConst()) return false; + + const Class* cls = clsTmp->getValClass(); + + if (!Transl::TargetCache::classIsPersistent(cls)) return false; + + // If the class requires initialization, it might not have been + // initialized yet. getSProp() below will trigger initialization, + // but that's only valid to do earlier if it doesn't require any + // property initializer ([sp]init methods). + if (cls->needInitialization()) return false; + + bool visible, accessible; + cls->getSProp(const_cast(ctx), propName, visible, accessible); + + return visible && accessible; +} + ////////////////////////////////////////////////////////////////////// template SSATmp* Simplifier::cns(Args&&... cns) { @@ -1391,41 +1427,25 @@ SSATmp* Simplifier::simplifyConvCellToBool(IRInstruction* inst) { } SSATmp* Simplifier::simplifyLdClsPropAddr(IRInstruction* inst) { - SSATmp* propName = inst->getSrc(1); + SSATmp* propName = inst->getSrc(1); if (!propName->isConst()) return nullptr; - SSATmp* cls = inst->getSrc(0); - const StringData* clsNameString = cls->isConst() - ? cls->getValClass()->preClass()->name() - : nullptr; - if (!clsNameString) { - // see if you can get the class name from a LdCls - IRInstruction* clsInst = cls->inst(); - if (clsInst->op() == LdCls || clsInst->op() == LdClsCached) { - SSATmp* clsName = clsInst->getSrc(0); - assert(clsName->isA(Type::Str)); - if (clsName->isConst()) { - clsNameString = clsName->getValStr(); - } - } - } - if (!clsNameString) return nullptr; + SSATmp* cls = inst->getSrc(0); + auto ctxCls = inst->getSrc(2)->getValClass(); - // We known both the class name and the property name statically so - // we can use the caching version of LdClsPropAddr. To avoid doing - // accessibility checks, we only do this if the context class is the - // same as the actual class the property is on. - auto const ctxCls = inst->getSrc(2)->getValClass(); - if (!ctxCls || !clsNameString->isame(ctxCls->preClass()->name())) { - return nullptr; + if (canUseSPropCache(cls, propName->getValStr(), ctxCls)) { + + const StringData* clsNameStr = findClassName(cls); + + return gen(LdClsPropAddrCached, + inst->getTaken(), + cls, + propName, + cns(clsNameStr), + inst->getSrc(2)); } - return gen(LdClsPropAddrCached, - inst->getTaken(), - cls, - propName, - cns(clsNameString), - inst->getSrc(2)); + return nullptr; } /* diff --git a/hphp/runtime/vm/translator/hopt/simplifier.h b/hphp/runtime/vm/translator/hopt/simplifier.h index fabac36de..cc9e47f0b 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.h +++ b/hphp/runtime/vm/translator/hopt/simplifier.h @@ -174,6 +174,14 @@ StackValueInfo getStackValue(SSATmp* stack, uint32_t index); */ void copyProp(IRInstruction*); +/* + * Checks if property propName of class clsTmp, called from context class ctx, + * can be accessed via the static property cache. + */ +bool canUseSPropCache(SSATmp* clsTmp, + const StringData* propName, + const Class* ctx); + ////////////////////////////////////////////////////////////////////// }}} diff --git a/hphp/runtime/vm/translator/targetcache.cpp b/hphp/runtime/vm/translator/targetcache.cpp index cb463d436..ba7126f40 100644 --- a/hphp/runtime/vm/translator/targetcache.cpp +++ b/hphp/runtime/vm/translator/targetcache.cpp @@ -234,6 +234,12 @@ bool isPersistentHandle(Handle handle) { return handle >= (unsigned)s_persistent_start; } +bool classIsPersistent(const Class* cls) { + return (RuntimeOption::RepoAuthoritative && + cls && + isPersistentHandle(cls->m_cachedOffset)); +} + static Handle allocLocked(bool persistent, int numBytes, int align) { s_handleMutex.assertOwnedBySelf(); align = Util::roundUpToPowerOfTwo(align); diff --git a/hphp/runtime/vm/translator/targetcache.h b/hphp/runtime/vm/translator/targetcache.h index 9103c6b2a..a00961273 100644 --- a/hphp/runtime/vm/translator/targetcache.h +++ b/hphp/runtime/vm/translator/targetcache.h @@ -105,6 +105,7 @@ bool testBit(size_t bit); bool testAndSetBit(CacheHandle handle, uint32_t mask); bool testAndSetBit(size_t bit); bool isPersistentHandle(CacheHandle handle); +bool classIsPersistent(const Class* cls); CacheHandle ptrToHandle(const void*); diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 5e6b13222..6640b2f92 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -258,13 +258,6 @@ struct IfCountNotStatic { } }; -bool -classIsPersistent(const Class* cls) { - return RuntimeOption::RepoAuthoritative && - cls && - TargetCache::isPersistentHandle(cls->m_cachedOffset); -} - bool classIsUnique(const Class* cls) { return RuntimeOption::RepoAuthoritative && diff --git a/hphp/runtime/vm/translator/translator-x64.h b/hphp/runtime/vm/translator/translator-x64.h index 435f6309b..f5d06c0c4 100644 --- a/hphp/runtime/vm/translator/translator-x64.h +++ b/hphp/runtime/vm/translator/translator-x64.h @@ -1185,7 +1185,6 @@ void dumpTranslationInfo(const Tracelet& t, TCA postGuards); bool classIsUnique(const Class* cls); bool classIsUniqueOrCtxParent(const Class* cls); bool classIsUniqueNormalClass(const Class* cls); -bool classIsPersistent(const Class* cls); // SpaceRecorder is used in translator-x64.cpp and in hopt/irtranslator.cpp // RAII logger for TC space consumption.