- Revision
- 189323
- Author
- [email protected]
- Date
- 2015-09-03 17:02:29 -0700 (Thu, 03 Sep 2015)
Log Message
StructureStubInfo should be able to reset itself without going through CodeBlock
https://bugs.webkit.org/show_bug.cgi?id=148743
Reviewed by Geoffrey Garen.
We had some resetStub...() methods in CodeBlock that didn't really do anything that
StructureStubInfo couldn't do by itself. It makes sense for the functionality to reset a
stub to be in the stub class, not in CodeBlock.
It's still true that:
- In order to mess with a StructureStubInfo, you either have to be in GC or you have to
be holding the owning CodeBlock's lock.
- StructureStubInfo doesn't remember which CodeBlock owns it (to save space), and all
of the callers of StructureStubInfo methods know which CodeBlock own it. So, many stub
methods take CodeBlock* as an argument.
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::finalizeUnconditionally):
(JSC::CodeBlock::addCallLinkInfo):
(JSC::CodeBlock::getCallLinkInfoForBytecodeIndex):
(JSC::CodeBlock::resetStub): Deleted.
(JSC::CodeBlock::resetStubInternal): Deleted.
(JSC::CodeBlock::resetStubDuringGCInternal): Deleted.
* bytecode/CodeBlock.h:
* bytecode/StructureStubClearingWatchpoint.cpp:
(JSC::StructureStubClearingWatchpoint::fireInternal):
* bytecode/StructureStubInfo.cpp:
(JSC::StructureStubInfo::deref):
(JSC::StructureStubInfo::reset):
(JSC::StructureStubInfo::visitWeakReferences):
* bytecode/StructureStubInfo.h:
(JSC::StructureStubInfo::initInList):
(JSC::StructureStubInfo::seenOnce):
(JSC::StructureStubInfo::reset): Deleted.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (189322 => 189323)
--- trunk/Source/_javascript_Core/ChangeLog 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-09-04 00:02:29 UTC (rev 189323)
@@ -1,3 +1,42 @@
+2015-09-03 Filip Pizlo <[email protected]>
+
+ StructureStubInfo should be able to reset itself without going through CodeBlock
+ https://bugs.webkit.org/show_bug.cgi?id=148743
+
+ Reviewed by Geoffrey Garen.
+
+ We had some resetStub...() methods in CodeBlock that didn't really do anything that
+ StructureStubInfo couldn't do by itself. It makes sense for the functionality to reset a
+ stub to be in the stub class, not in CodeBlock.
+
+ It's still true that:
+
+ - In order to mess with a StructureStubInfo, you either have to be in GC or you have to
+ be holding the owning CodeBlock's lock.
+
+ - StructureStubInfo doesn't remember which CodeBlock owns it (to save space), and all
+ of the callers of StructureStubInfo methods know which CodeBlock own it. So, many stub
+ methods take CodeBlock* as an argument.
+
+ * bytecode/CodeBlock.cpp:
+ (JSC::CodeBlock::finalizeUnconditionally):
+ (JSC::CodeBlock::addCallLinkInfo):
+ (JSC::CodeBlock::getCallLinkInfoForBytecodeIndex):
+ (JSC::CodeBlock::resetStub): Deleted.
+ (JSC::CodeBlock::resetStubInternal): Deleted.
+ (JSC::CodeBlock::resetStubDuringGCInternal): Deleted.
+ * bytecode/CodeBlock.h:
+ * bytecode/StructureStubClearingWatchpoint.cpp:
+ (JSC::StructureStubClearingWatchpoint::fireInternal):
+ * bytecode/StructureStubInfo.cpp:
+ (JSC::StructureStubInfo::deref):
+ (JSC::StructureStubInfo::reset):
+ (JSC::StructureStubInfo::visitWeakReferences):
+ * bytecode/StructureStubInfo.h:
+ (JSC::StructureStubInfo::initInList):
+ (JSC::StructureStubInfo::seenOnce):
+ (JSC::StructureStubInfo::reset): Deleted.
+
2015-09-03 Sukolsak Sakshuwong <[email protected]>
Implement some arithmetic instructions in WebAssembly
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp (189322 => 189323)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.cpp 2015-09-04 00:02:29 UTC (rev 189323)
@@ -2690,11 +2690,7 @@
for (Bag<StructureStubInfo>::iterator iter = m_stubInfos.begin(); !!iter; ++iter) {
StructureStubInfo& stubInfo = **iter;
-
- if (stubInfo.visitWeakReferences(*vm()))
- continue;
-
- resetStubDuringGCInternal(stubInfo);
+ stubInfo.visitWeakReferences(this);
}
}
#endif
@@ -2774,46 +2770,6 @@
return m_callLinkInfos.add();
}
-void CodeBlock::resetStub(StructureStubInfo& stubInfo)
-{
- if (stubInfo.accessType == access_unset)
- return;
-
- ConcurrentJITLocker locker(m_lock);
-
- resetStubInternal(stubInfo);
-}
-
-void CodeBlock::resetStubInternal(StructureStubInfo& stubInfo)
-{
- AccessType accessType = static_cast<AccessType>(stubInfo.accessType);
-
- if (Options::verboseOSR()) {
- // This can be called from GC destructor calls, so we don't try to do a full dump
- // of the CodeBlock.
- dataLog("Clearing structure cache (kind ", static_cast<int>(stubInfo.accessType), ") in ", RawPointer(this), ".\n");
- }
-
- RELEASE_ASSERT(JITCode::isJIT(jitType()));
-
- if (isGetByIdAccess(accessType))
- resetGetByID(this, stubInfo);
- else if (isPutByIdAccess(accessType))
- resetPutByID(this, stubInfo);
- else {
- RELEASE_ASSERT(isInAccess(accessType));
- resetIn(this, stubInfo);
- }
-
- stubInfo.reset();
-}
-
-void CodeBlock::resetStubDuringGCInternal(StructureStubInfo& stubInfo)
-{
- resetStubInternal(stubInfo);
- stubInfo.resetByGC = true;
-}
-
CallLinkInfo* CodeBlock::getCallLinkInfoForBytecodeIndex(unsigned index)
{
for (auto iter = m_callLinkInfos.begin(); !!iter; ++iter) {
Modified: trunk/Source/_javascript_Core/bytecode/CodeBlock.h (189322 => 189323)
--- trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/bytecode/CodeBlock.h 2015-09-04 00:02:29 UTC (rev 189323)
@@ -215,8 +215,6 @@
// stub info.
StructureStubInfo* findStubInfo(CodeOrigin);
- void resetStub(StructureStubInfo&);
-
ByValInfo* addByValInfo();
CallLinkInfo* addCallLinkInfo();
@@ -980,10 +978,6 @@
void insertBasicBlockBoundariesForControlFlowProfiler(Vector<Instruction, 0, UnsafeVectorOverflow>&);
-#if ENABLE(JIT)
- void resetStubInternal(StructureStubInfo&);
- void resetStubDuringGCInternal(StructureStubInfo&);
-#endif
WriteBarrier<UnlinkedCodeBlock> m_unlinkedCode;
int m_numParameters;
union {
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp (189322 => 189323)
--- trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubClearingWatchpoint.cpp 2015-09-04 00:02:29 UTC (rev 189323)
@@ -51,7 +51,8 @@
// This will implicitly cause my own demise: stub reset removes all watchpoints.
// That works, because deleting a watchpoint removes it from the set's list, and
// the set's list traversal for firing is robust against the set changing.
- m_holder.codeBlock()->resetStub(*m_holder.stubInfo());
+ ConcurrentJITLocker locker(m_holder.codeBlock()->m_lock);
+ m_holder.stubInfo()->reset(m_holder.codeBlock());
return;
}
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp (189322 => 189323)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.cpp 2015-09-04 00:02:29 UTC (rev 189323)
@@ -29,6 +29,7 @@
#include "JSObject.h"
#include "PolymorphicGetByIdList.h"
#include "PolymorphicPutByIdList.h"
+#include "Repatch.h"
namespace JSC {
@@ -63,46 +64,75 @@
}
}
-bool StructureStubInfo::visitWeakReferences(VM& vm)
+void StructureStubInfo::reset(CodeBlock* codeBlock)
{
+ if (accessType == access_unset)
+ return;
+
+ if (Options::verboseOSR()) {
+ // This can be called from GC destructor calls, so we don't try to do a full dump
+ // of the CodeBlock.
+ dataLog("Clearing structure cache (kind ", static_cast<int>(accessType), ") in ", RawPointer(codeBlock), ".\n");
+ }
+
+ if (isGetByIdAccess(static_cast<AccessType>(accessType)))
+ resetGetByID(codeBlock, *this);
+ else if (isPutByIdAccess(static_cast<AccessType>(accessType)))
+ resetPutByID(codeBlock, *this);
+ else {
+ RELEASE_ASSERT(isInAccess(static_cast<AccessType>(accessType)));
+ resetIn(codeBlock, *this);
+ }
+
+ deref();
+ accessType = access_unset;
+ stubRoutine = nullptr;
+ watchpoints = nullptr;
+}
+
+void StructureStubInfo::visitWeakReferences(CodeBlock* codeBlock)
+{
+ VM& vm = *codeBlock->vm();
+
switch (accessType) {
case access_get_by_id_self:
- if (!Heap::isMarked(u.getByIdSelf.baseObjectStructure.get()))
- return false;
+ if (Heap::isMarked(u.getByIdSelf.baseObjectStructure.get()))
+ return;
break;
case access_get_by_id_list: {
- if (!u.getByIdList.list->visitWeak(vm))
- return false;
+ if (u.getByIdList.list->visitWeak(vm))
+ return;
break;
}
case access_put_by_id_transition_normal:
case access_put_by_id_transition_direct:
- if (!Heap::isMarked(u.putByIdTransition.previousStructure.get())
- || !Heap::isMarked(u.putByIdTransition.structure.get()))
- return false;
- if (!ObjectPropertyConditionSet::fromRawPointer(u.putByIdTransition.rawConditionSet).areStillLive())
- return false;
+ if (Heap::isMarked(u.putByIdTransition.previousStructure.get())
+ && Heap::isMarked(u.putByIdTransition.structure.get())
+ && ObjectPropertyConditionSet::fromRawPointer(u.putByIdTransition.rawConditionSet).areStillLive())
+ return;
break;
case access_put_by_id_replace:
- if (!Heap::isMarked(u.putByIdReplace.baseObjectStructure.get()))
- return false;
+ if (Heap::isMarked(u.putByIdReplace.baseObjectStructure.get()))
+ return;
break;
case access_put_by_id_list:
- if (!u.putByIdList.list->visitWeak(vm))
- return false;
+ if (u.putByIdList.list->visitWeak(vm))
+ return;
break;
case access_in_list: {
PolymorphicAccessStructureList* polymorphicStructures = u.inList.structureList;
- if (!polymorphicStructures->visitWeak(u.inList.listSize))
- return false;
+ if (polymorphicStructures->visitWeak(u.inList.listSize))
+ return;
break;
}
default:
// The rest of the instructions don't require references, so there is no need to
// do anything.
- break;
+ return;
}
- return true;
+
+ reset(codeBlock);
+ resetByGC = true;
}
#endif
Modified: trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h (189322 => 189323)
--- trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/_javascript_Core/bytecode/StructureStubInfo.h 2015-09-04 00:02:29 UTC (rev 189323)
@@ -147,25 +147,13 @@
u.inList.listSize = listSize;
}
- void reset()
- {
- deref();
- accessType = access_unset;
- stubRoutine = nullptr;
- watchpoints = nullptr;
- }
+ void reset(CodeBlock*);
void deref();
- // Check if the stub has weak references that are dead. If there are dead ones that imply
- // that the stub should be entirely reset, this should return false. If there are dead ones
- // that can be handled internally by the stub and don't require a full reset, then this
- // should reset them and return true. If there are no dead weak references, return true.
- // If this method returns true it means that it has left the stub in a state where all
- // outgoing GC pointers are known to point to currently marked objects; this method is
- // allowed to accomplish this by either clearing those pointers somehow or by proving that
- // they have already been marked. It is not allowed to mark new objects.
- bool visitWeakReferences(VM&);
+ // Check if the stub has weak references that are dead. If it does, then it resets itself,
+ // either entirely or just enough to ensure that those dead pointers don't get used anymore.
+ void visitWeakReferences(CodeBlock*);
bool seenOnce()
{
