<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[189323] trunk/Source/JavaScriptCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/189323">189323</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-09-03 17:02:29 -0700 (Thu, 03 Sep 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCodeBlockcpp">trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeCodeBlockh">trunk/Source/JavaScriptCore/bytecode/CodeBlock.h</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeStructureStubClearingWatchpointcpp">trunk/Source/JavaScriptCore/bytecode/StructureStubClearingWatchpoint.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeStructureStubInfocpp">trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCorebytecodeStructureStubInfoh">trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -1,3 +1,42 @@
</span><ins>+2015-09-03 Filip Pizlo <fpizlo@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2015-09-03 Sukolsak Sakshuwong <sukolsak@gmail.com>
</span><span class="cx">
</span><span class="cx"> Implement some arithmetic instructions in WebAssembly
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCodeBlockcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -2690,11 +2690,7 @@
</span><span class="cx">
</span><span class="cx"> for (Bag<StructureStubInfo>::iterator iter = m_stubInfos.begin(); !!iter; ++iter) {
</span><span class="cx"> StructureStubInfo& stubInfo = **iter;
</span><del>-
- if (stubInfo.visitWeakReferences(*vm()))
- continue;
-
- resetStubDuringGCInternal(stubInfo);
</del><ins>+ stubInfo.visitWeakReferences(this);
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="lines">@@ -2774,46 +2770,6 @@
</span><span class="cx"> return m_callLinkInfos.add();
</span><span class="cx"> }
</span><span class="cx">
</span><del>-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;
-}
-
</del><span class="cx"> CallLinkInfo* CodeBlock::getCallLinkInfoForBytecodeIndex(unsigned index)
</span><span class="cx"> {
</span><span class="cx"> for (auto iter = m_callLinkInfos.begin(); !!iter; ++iter) {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeCodeBlockh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/CodeBlock.h (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/CodeBlock.h 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/bytecode/CodeBlock.h 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -215,8 +215,6 @@
</span><span class="cx"> // stub info.
</span><span class="cx"> StructureStubInfo* findStubInfo(CodeOrigin);
</span><span class="cx">
</span><del>- void resetStub(StructureStubInfo&);
-
</del><span class="cx"> ByValInfo* addByValInfo();
</span><span class="cx">
</span><span class="cx"> CallLinkInfo* addCallLinkInfo();
</span><span class="lines">@@ -980,10 +978,6 @@
</span><span class="cx">
</span><span class="cx"> void insertBasicBlockBoundariesForControlFlowProfiler(Vector<Instruction, 0, UnsafeVectorOverflow>&);
</span><span class="cx">
</span><del>-#if ENABLE(JIT)
- void resetStubInternal(StructureStubInfo&);
- void resetStubDuringGCInternal(StructureStubInfo&);
-#endif
</del><span class="cx"> WriteBarrier<UnlinkedCodeBlock> m_unlinkedCode;
</span><span class="cx"> int m_numParameters;
</span><span class="cx"> union {
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeStructureStubClearingWatchpointcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/StructureStubClearingWatchpoint.cpp (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/StructureStubClearingWatchpoint.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/bytecode/StructureStubClearingWatchpoint.cpp 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -51,7 +51,8 @@
</span><span class="cx"> // This will implicitly cause my own demise: stub reset removes all watchpoints.
</span><span class="cx"> // That works, because deleting a watchpoint removes it from the set's list, and
</span><span class="cx"> // the set's list traversal for firing is robust against the set changing.
</span><del>- m_holder.codeBlock()->resetStub(*m_holder.stubInfo());
</del><ins>+ ConcurrentJITLocker locker(m_holder.codeBlock()->m_lock);
+ m_holder.stubInfo()->reset(m_holder.codeBlock());
</ins><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeStructureStubInfocpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.cpp 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -29,6 +29,7 @@
</span><span class="cx"> #include "JSObject.h"
</span><span class="cx"> #include "PolymorphicGetByIdList.h"
</span><span class="cx"> #include "PolymorphicPutByIdList.h"
</span><ins>+#include "Repatch.h"
</ins><span class="cx">
</span><span class="cx"> namespace JSC {
</span><span class="cx">
</span><span class="lines">@@ -63,46 +64,75 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-bool StructureStubInfo::visitWeakReferences(VM& vm)
</del><ins>+void StructureStubInfo::reset(CodeBlock* codeBlock)
</ins><span class="cx"> {
</span><ins>+ 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();
+
</ins><span class="cx"> switch (accessType) {
</span><span class="cx"> case access_get_by_id_self:
</span><del>- if (!Heap::isMarked(u.getByIdSelf.baseObjectStructure.get()))
- return false;
</del><ins>+ if (Heap::isMarked(u.getByIdSelf.baseObjectStructure.get()))
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> case access_get_by_id_list: {
</span><del>- if (!u.getByIdList.list->visitWeak(vm))
- return false;
</del><ins>+ if (u.getByIdList.list->visitWeak(vm))
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> case access_put_by_id_transition_normal:
</span><span class="cx"> case access_put_by_id_transition_direct:
</span><del>- if (!Heap::isMarked(u.putByIdTransition.previousStructure.get())
- || !Heap::isMarked(u.putByIdTransition.structure.get()))
- return false;
- if (!ObjectPropertyConditionSet::fromRawPointer(u.putByIdTransition.rawConditionSet).areStillLive())
- return false;
</del><ins>+ if (Heap::isMarked(u.putByIdTransition.previousStructure.get())
+ && Heap::isMarked(u.putByIdTransition.structure.get())
+ && ObjectPropertyConditionSet::fromRawPointer(u.putByIdTransition.rawConditionSet).areStillLive())
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> case access_put_by_id_replace:
</span><del>- if (!Heap::isMarked(u.putByIdReplace.baseObjectStructure.get()))
- return false;
</del><ins>+ if (Heap::isMarked(u.putByIdReplace.baseObjectStructure.get()))
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> case access_put_by_id_list:
</span><del>- if (!u.putByIdList.list->visitWeak(vm))
- return false;
</del><ins>+ if (u.putByIdList.list->visitWeak(vm))
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> case access_in_list: {
</span><span class="cx"> PolymorphicAccessStructureList* polymorphicStructures = u.inList.structureList;
</span><del>- if (!polymorphicStructures->visitWeak(u.inList.listSize))
- return false;
</del><ins>+ if (polymorphicStructures->visitWeak(u.inList.listSize))
+ return;
</ins><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx"> default:
</span><span class="cx"> // The rest of the instructions don't require references, so there is no need to
</span><span class="cx"> // do anything.
</span><del>- break;
</del><ins>+ return;
</ins><span class="cx"> }
</span><del>- return true;
</del><ins>+
+ reset(codeBlock);
+ resetByGC = true;
</ins><span class="cx"> }
</span><span class="cx"> #endif
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCorebytecodeStructureStubInfoh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h (189322 => 189323)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h 2015-09-03 23:53:33 UTC (rev 189322)
+++ trunk/Source/JavaScriptCore/bytecode/StructureStubInfo.h 2015-09-04 00:02:29 UTC (rev 189323)
</span><span class="lines">@@ -147,25 +147,13 @@
</span><span class="cx"> u.inList.listSize = listSize;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- void reset()
- {
- deref();
- accessType = access_unset;
- stubRoutine = nullptr;
- watchpoints = nullptr;
- }
</del><ins>+ void reset(CodeBlock*);
</ins><span class="cx">
</span><span class="cx"> void deref();
</span><span class="cx">
</span><del>- // 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&);
</del><ins>+ // 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*);
</ins><span class="cx">
</span><span class="cx"> bool seenOnce()
</span><span class="cx"> {
</span></span></pre>
</div>
</div>
</body>
</html>