Commit abe6d6e4 authored by barraclough@apple.com's avatar barraclough@apple.com

Some cleanup in PropertySlot

https://bugs.webkit.org/show_bug.cgi?id=119189

Reviewed by Geoff Garen.

PropertySlot represents a property in one of four states - value, getter, custom, or custom-index.
The state is currently tracked redundantly by two mechanisms - the custom getter function (m_getValue)
is set to a special value to indicate the type (other than custom), and the type is also tracked by
an enum - but only if cacheable. Cacheability can typically be determined by the value of m_offset
(this is invalidOffset if not cacheable).

    * Internally, always track the type of the property using an enum value, PropertyType.
    * Use m_offset to indicate cacheable.
    * Keep the external interface (CachedPropertyType) unchanged.
    * Better pack data into the m_data union.

Performance neutral.

* dfg/DFGRepatch.cpp:
(JSC::DFG::tryCacheGetByID):
(JSC::DFG::tryBuildGetByIDList):
    - cachedPropertyType() -> isCacheable*()
* jit/JITPropertyAccess.cpp:
(JSC::JIT::privateCompileGetByIdProto):
(JSC::JIT::privateCompileGetByIdSelfList):
(JSC::JIT::privateCompileGetByIdProtoList):
(JSC::JIT::privateCompileGetByIdChainList):
(JSC::JIT::privateCompileGetByIdChain):
    - cachedPropertyType() -> isCacheable*()
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::privateCompileGetByIdProto):
(JSC::JIT::privateCompileGetByIdSelfList):
(JSC::JIT::privateCompileGetByIdProtoList):
(JSC::JIT::privateCompileGetByIdChainList):
(JSC::JIT::privateCompileGetByIdChain):
    - cachedPropertyType() -> isCacheable*()
* jit/JITStubs.cpp:
(JSC::tryCacheGetByID):
    - cachedPropertyType() -> isCacheable*()
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
    - cachedPropertyType() -> isCacheable*()
* runtime/PropertySlot.cpp:
(JSC::PropertySlot::functionGetter):
    - refactoring described above.
* runtime/PropertySlot.h:
(JSC::PropertySlot::PropertySlot):
(JSC::PropertySlot::getValue):
(JSC::PropertySlot::isCacheable):
(JSC::PropertySlot::isCacheableValue):
(JSC::PropertySlot::isCacheableGetter):
(JSC::PropertySlot::isCacheableCustom):
(JSC::PropertySlot::cachedOffset):
(JSC::PropertySlot::customGetter):
(JSC::PropertySlot::setValue):
(JSC::PropertySlot::setCustom):
(JSC::PropertySlot::setCacheableCustom):
(JSC::PropertySlot::setCustomIndex):
(JSC::PropertySlot::setGetterSlot):
(JSC::PropertySlot::setCacheableGetterSlot):
(JSC::PropertySlot::setUndefined):
(JSC::PropertySlot::slotBase):
(JSC::PropertySlot::setBase):
    - refactoring described above.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@153454 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 45388cff
2013-07-29 Gavin Barraclough <barraclough@apple.com>
Some cleanup in PropertySlot
https://bugs.webkit.org/show_bug.cgi?id=119189
Reviewed by Geoff Garen.
PropertySlot represents a property in one of four states - value, getter, custom, or custom-index.
The state is currently tracked redundantly by two mechanisms - the custom getter function (m_getValue)
is set to a special value to indicate the type (other than custom), and the type is also tracked by
an enum - but only if cacheable. Cacheability can typically be determined by the value of m_offset
(this is invalidOffset if not cacheable).
* Internally, always track the type of the property using an enum value, PropertyType.
* Use m_offset to indicate cacheable.
* Keep the external interface (CachedPropertyType) unchanged.
* Better pack data into the m_data union.
Performance neutral.
* dfg/DFGRepatch.cpp:
(JSC::DFG::tryCacheGetByID):
(JSC::DFG::tryBuildGetByIDList):
- cachedPropertyType() -> isCacheable*()
* jit/JITPropertyAccess.cpp:
(JSC::JIT::privateCompileGetByIdProto):
(JSC::JIT::privateCompileGetByIdSelfList):
(JSC::JIT::privateCompileGetByIdProtoList):
(JSC::JIT::privateCompileGetByIdChainList):
(JSC::JIT::privateCompileGetByIdChain):
- cachedPropertyType() -> isCacheable*()
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::privateCompileGetByIdProto):
(JSC::JIT::privateCompileGetByIdSelfList):
(JSC::JIT::privateCompileGetByIdProtoList):
(JSC::JIT::privateCompileGetByIdChainList):
(JSC::JIT::privateCompileGetByIdChain):
- cachedPropertyType() -> isCacheable*()
* jit/JITStubs.cpp:
(JSC::tryCacheGetByID):
- cachedPropertyType() -> isCacheable*()
* llint/LLIntSlowPaths.cpp:
(JSC::LLInt::LLINT_SLOW_PATH_DECL):
- cachedPropertyType() -> isCacheable*()
* runtime/PropertySlot.cpp:
(JSC::PropertySlot::functionGetter):
- refactoring described above.
* runtime/PropertySlot.h:
(JSC::PropertySlot::PropertySlot):
(JSC::PropertySlot::getValue):
(JSC::PropertySlot::isCacheable):
(JSC::PropertySlot::isCacheableValue):
(JSC::PropertySlot::isCacheableGetter):
(JSC::PropertySlot::isCacheableCustom):
(JSC::PropertySlot::cachedOffset):
(JSC::PropertySlot::customGetter):
(JSC::PropertySlot::setValue):
(JSC::PropertySlot::setCustom):
(JSC::PropertySlot::setCacheableCustom):
(JSC::PropertySlot::setCustomIndex):
(JSC::PropertySlot::setGetterSlot):
(JSC::PropertySlot::setCacheableGetterSlot):
(JSC::PropertySlot::setUndefined):
(JSC::PropertySlot::slotBase):
(JSC::PropertySlot::setBase):
- refactoring described above.
2013-07-28 Oliver Hunt <oliver@apple.com>
REGRESSION: Crash when opening Facebook.com
......
......@@ -324,7 +324,7 @@ static bool tryCacheGetByID(ExecState* exec, JSValue baseValue, const Identifier
// Optimize self access.
if (slot.slotBase() == baseValue) {
if ((slot.cachedPropertyType() != PropertySlot::Value)
if (!slot.isCacheableValue()
|| !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset()))) {
dfgRepatchCall(codeBlock, stubInfo.callReturnLocation, operationGetByIdBuildList);
return true;
......@@ -339,7 +339,7 @@ static bool tryCacheGetByID(ExecState* exec, JSValue baseValue, const Identifier
return false;
// FIXME: optimize getters and setters
if (slot.cachedPropertyType() != PropertySlot::Value)
if (!slot.isCacheableValue())
return false;
PropertyOffset offset = slot.cachedOffset();
......@@ -438,7 +438,7 @@ static bool tryBuildGetByIDList(ExecState* exec, JSValue baseValue, const Identi
// We cannot do as much inline caching if the registers were not flushed prior to this GetById. In particular,
// non-Value cached properties require planting calls, which requires registers to have been flushed. Thus,
// if registers were not flushed, don't do non-Value caching.
if (slot.cachedPropertyType() != PropertySlot::Value)
if (!slot.isCacheableValue())
return false;
}
......@@ -475,9 +475,8 @@ static bool tryBuildGetByIDList(ExecState* exec, JSValue baseValue, const Identi
FunctionPtr operationFunction;
MacroAssembler::Jump success;
if (slot.cachedPropertyType() == PropertySlot::Getter
|| slot.cachedPropertyType() == PropertySlot::Custom) {
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter() || slot.isCacheableCustom()) {
if (slot.isCacheableGetter()) {
ASSERT(scratchGPR != InvalidGPRReg);
ASSERT(baseGPR != scratchGPR);
if (isInlineOffset(slot.cachedOffset())) {
......@@ -567,8 +566,7 @@ static bool tryBuildGetByIDList(ExecState* exec, JSValue baseValue, const Identi
stubInfo.patch.dfg.deltaCallToDone).executableAddress())),
*vm,
codeBlock->ownerExecutable(),
slot.cachedPropertyType() == PropertySlot::Getter
|| slot.cachedPropertyType() == PropertySlot::Custom);
slot.isCacheableGetter() || slot.isCacheableCustom());
polymorphicStructureList->list[listIndex].set(*vm, codeBlock->ownerExecutable(), stubRoutine, structure, isDirect);
......@@ -576,8 +574,7 @@ static bool tryBuildGetByIDList(ExecState* exec, JSValue baseValue, const Identi
return listIndex < (POLYMORPHIC_LIST_CACHE_SIZE - 1);
}
if (baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching()
|| slot.cachedPropertyType() != PropertySlot::Value)
if (baseValue.asCell()->structure()->typeInfo().prohibitsPropertyCaching() || !slot.isCacheableValue())
return false;
ASSERT(slot.slotBase().isObject());
......
......@@ -856,7 +856,7 @@ void JIT::privateCompileGetByIdProto(StructureStubInfo* stubInfo, Structure* str
bool needsStubLink = false;
// Checks out okay!
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -864,7 +864,7 @@ void JIT::privateCompileGetByIdProto(StructureStubInfo* stubInfo, Structure* str
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -917,7 +917,7 @@ void JIT::privateCompileGetByIdSelfList(StructureStubInfo* stubInfo, Polymorphic
Jump failureCase = checkStructure(regT0, structure);
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(regT0, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -925,7 +925,7 @@ void JIT::privateCompileGetByIdSelfList(StructureStubInfo* stubInfo, Polymorphic
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(regT0);
......@@ -991,7 +991,7 @@ void JIT::privateCompileGetByIdProtoList(StructureStubInfo* stubInfo, Polymorphi
// Checks out okay!
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -999,7 +999,7 @@ void JIT::privateCompileGetByIdProtoList(StructureStubInfo* stubInfo, Polymorphi
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -1070,7 +1070,7 @@ void JIT::privateCompileGetByIdChainList(StructureStubInfo* stubInfo, Polymorphi
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -1078,7 +1078,7 @@ void JIT::privateCompileGetByIdChainList(StructureStubInfo* stubInfo, Polymorphi
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -1148,7 +1148,7 @@ void JIT::privateCompileGetByIdChain(StructureStubInfo* stubInfo, Structure* str
ASSERT(protoObject);
bool needsStubLink = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -1156,7 +1156,7 @@ void JIT::privateCompileGetByIdChain(StructureStubInfo* stubInfo, Structure* str
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......
......@@ -818,7 +818,7 @@ void JIT::privateCompileGetByIdProto(StructureStubInfo* stubInfo, Structure* str
bool needsStubLink = false;
// Checks out okay!
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT2, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -826,7 +826,7 @@ void JIT::privateCompileGetByIdProto(StructureStubInfo* stubInfo, Structure* str
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -884,7 +884,7 @@ void JIT::privateCompileGetByIdSelfList(StructureStubInfo* stubInfo, Polymorphic
Jump failureCase = checkStructure(regT0, structure);
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(regT0, regT2, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -892,7 +892,7 @@ void JIT::privateCompileGetByIdSelfList(StructureStubInfo* stubInfo, Polymorphic
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(regT0);
......@@ -958,7 +958,7 @@ void JIT::privateCompileGetByIdProtoList(StructureStubInfo* stubInfo, Polymorphi
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT2, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -966,7 +966,7 @@ void JIT::privateCompileGetByIdProtoList(StructureStubInfo* stubInfo, Polymorphi
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -1037,7 +1037,7 @@ void JIT::privateCompileGetByIdChainList(StructureStubInfo* stubInfo, Polymorphi
bool needsStubLink = false;
bool isDirect = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT2, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -1045,7 +1045,7 @@ void JIT::privateCompileGetByIdChainList(StructureStubInfo* stubInfo, Polymorphi
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......@@ -1115,7 +1115,7 @@ void JIT::privateCompileGetByIdChain(StructureStubInfo* stubInfo, Structure* str
ASSERT(protoObject);
bool needsStubLink = false;
if (slot.cachedPropertyType() == PropertySlot::Getter) {
if (slot.isCacheableGetter()) {
needsStubLink = true;
compileGetDirectOffset(protoObject, regT2, regT1, cachedOffset);
JITStubCall stubCall(this, cti_op_get_by_id_getter_stub);
......@@ -1123,7 +1123,7 @@ void JIT::privateCompileGetByIdChain(StructureStubInfo* stubInfo, Structure* str
stubCall.addArgument(regT0);
stubCall.addArgument(TrustedImmPtr(stubInfo->callReturnLocation.executableAddress()));
stubCall.call();
} else if (slot.cachedPropertyType() == PropertySlot::Custom) {
} else if (slot.isCacheableCustom()) {
needsStubLink = true;
JITStubCall stubCall(this, cti_op_get_by_id_custom_stub);
stubCall.addArgument(TrustedImmPtr(protoObject));
......
......@@ -220,8 +220,7 @@ NEVER_INLINE static void tryCacheGetByID(CallFrame* callFrame, CodeBlock* codeBl
if (slot.slotBase() == baseValue) {
RELEASE_ASSERT(stubInfo->accessType == access_unset);
if ((slot.cachedPropertyType() != PropertySlot::Value)
|| !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset())))
if (!slot.isCacheableValue() || !MacroAssembler::isCompactPtrAlignedAddressOffset(maxOffsetRelativeToPatchedStorage(slot.cachedOffset())))
ctiPatchCallByReturnAddress(codeBlock, returnAddress, FunctionPtr(cti_op_get_by_id_self_fail));
else {
JIT::patchGetByIdSelf(codeBlock, stubInfo, structure, slot.cachedOffset(), returnAddress);
......@@ -255,7 +254,7 @@ NEVER_INLINE static void tryCacheGetByID(CallFrame* callFrame, CodeBlock* codeBl
offset = slotBaseObject->structure()->get(callFrame->vm(), propertyName);
}
stubInfo->initGetByIdProto(callFrame->vm(), codeBlock->ownerExecutable(), structure, slotBaseObject->structure(), slot.cachedPropertyType() == PropertySlot::Value);
stubInfo->initGetByIdProto(callFrame->vm(), codeBlock->ownerExecutable(), structure, slotBaseObject->structure(), slot.isCacheableValue());
ASSERT(!structure->isDictionary());
ASSERT(!slotBaseObject->structure()->isDictionary());
......@@ -272,7 +271,7 @@ NEVER_INLINE static void tryCacheGetByID(CallFrame* callFrame, CodeBlock* codeBl
}
StructureChain* prototypeChain = structure->prototypeChain(callFrame);
stubInfo->initGetByIdChain(callFrame->vm(), codeBlock->ownerExecutable(), structure, prototypeChain, count, slot.cachedPropertyType() == PropertySlot::Value);
stubInfo->initGetByIdChain(callFrame->vm(), codeBlock->ownerExecutable(), structure, prototypeChain, count, slot.isCacheableValue());
JIT::compileGetByIdChain(callFrame->scope()->vm(), callFrame, codeBlock, stubInfo, structure, prototypeChain, count, propertyName, slot, offset, returnAddress);
}
......
......@@ -515,7 +515,7 @@ LLINT_SLOW_PATH_DECL(slow_path_get_by_id)
&& baseValue.isCell()
&& slot.isCacheable()
&& slot.slotBase() == baseValue
&& slot.cachedPropertyType() == PropertySlot::Value) {
&& slot.isCacheableValue()) {
JSCell* baseCell = baseValue.asCell();
Structure* structure = baseCell->structure();
......
......@@ -28,7 +28,7 @@ namespace JSC {
JSValue PropertySlot::functionGetter(ExecState* exec) const
{
return callGetter(exec, m_thisValue, m_data.getterSetter);
return callGetter(exec, JSValue::decode(m_data.getter.thisValue), m_data.getter.getterSetter);
}
} // namespace JSC
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment