<!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>[181019] 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/181019">181019</a></dd>
<dt>Author</dt> <dd>fpizlo@apple.com</dd>
<dt>Date</dt> <dd>2015-03-04 13:32:27 -0800 (Wed, 04 Mar 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>Only Heap should be in charge of deciding how to select a subspace for a type
https://bugs.webkit.org/show_bug.cgi?id=142304
Reviewed by Mark Lam.
This slightly reduces the code duplication for selecting subspace based on type, and what
duplication is left is at least localized in HeapInlines.h. The immediate effect is that
the DFG and FTL don't have to duplicate this pattern.
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
(JSC::DFG::SpeculativeJIT::emitAllocateVariableSizedJSObject):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::LowerDFGToLLVM::allocateObject):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::allocateObjectOfType):
(JSC::Heap::subspaceForObjectOfType):
(JSC::Heap::allocatorForObjectOfType):
* runtime/JSCellInlines.h:
(JSC::allocateCell):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceJavaScriptCoreChangeLog">trunk/Source/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkSourceJavaScriptCoredfgDFGSpeculativeJITh">trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp">trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeaph">trunk/Source/JavaScriptCore/heap/Heap.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreheapHeapInlinesh">trunk/Source/JavaScriptCore/heap/HeapInlines.h</a></li>
<li><a href="#trunkSourceJavaScriptCoreruntimeJSCellInlinesh">trunk/Source/JavaScriptCore/runtime/JSCellInlines.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ChangeLog (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ChangeLog 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/ChangeLog 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -1,3 +1,27 @@
</span><ins>+2015-03-04 Filip Pizlo <fpizlo@apple.com>
+
+ Only Heap should be in charge of deciding how to select a subspace for a type
+ https://bugs.webkit.org/show_bug.cgi?id=142304
+
+ Reviewed by Mark Lam.
+
+ This slightly reduces the code duplication for selecting subspace based on type, and what
+ duplication is left is at least localized in HeapInlines.h. The immediate effect is that
+ the DFG and FTL don't have to duplicate this pattern.
+
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::emitAllocateJSObject):
+ (JSC::DFG::SpeculativeJIT::emitAllocateVariableSizedJSObject):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::LowerDFGToLLVM::allocateObject):
+ * heap/Heap.h:
+ * heap/HeapInlines.h:
+ (JSC::Heap::allocateObjectOfType):
+ (JSC::Heap::subspaceForObjectOfType):
+ (JSC::Heap::allocatorForObjectOfType):
+ * runtime/JSCellInlines.h:
+ (JSC::allocateCell):
+
</ins><span class="cx"> 2015-03-04 Andreas Kling <akling@apple.com>
</span><span class="cx">
</span><span class="cx"> Stale entries in WeakGCMaps are keeping tons of WeakBlocks alive unnecessarily.
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoredfgDFGSpeculativeJITh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -2258,14 +2258,8 @@
</span><span class="cx"> void emitAllocateJSObject(GPRReg resultGPR, StructureType structure, StorageType storage,
</span><span class="cx"> GPRReg scratchGPR1, GPRReg scratchGPR2, MacroAssembler::JumpList& slowPath)
</span><span class="cx"> {
</span><del>- MarkedAllocator* allocator = 0;
</del><span class="cx"> size_t size = ClassType::allocationSize(0);
</span><del>- if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
- allocator = &m_jit.vm()->heap.allocatorForObjectWithImmortalStructureDestructor(size);
- else if (ClassType::needsDestruction)
- allocator = &m_jit.vm()->heap.allocatorForObjectWithNormalDestructor(size);
- else
- allocator = &m_jit.vm()->heap.allocatorForObjectWithoutDestructor(size);
</del><ins>+ MarkedAllocator* allocator = &m_jit.vm()->heap.allocatorForObjectOfType<ClassType>(size);
</ins><span class="cx"> m_jit.move(TrustedImmPtr(allocator), scratchGPR1);
</span><span class="cx"> emitAllocateJSObject(resultGPR, scratchGPR1, structure, storage, scratchGPR2, slowPath);
</span><span class="cx"> }
</span><span class="lines">@@ -2276,25 +2270,19 @@
</span><span class="cx"> static_assert(!(MarkedSpace::preciseStep & (MarkedSpace::preciseStep - 1)), "MarkedSpace::preciseStep must be a power of two.");
</span><span class="cx"> static_assert(!(MarkedSpace::impreciseStep & (MarkedSpace::impreciseStep - 1)), "MarkedSpace::impreciseStep must be a power of two.");
</span><span class="cx">
</span><del>- MarkedSpace::Subspace* subspace;
- if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
- subspace = &m_jit.vm()->heap.subspaceForObjectsWithImmortalStructure();
- else if (ClassType::needsDestruction)
- subspace = &m_jit.vm()->heap.subspaceForObjectNormalDestructor();
- else
- subspace = &m_jit.vm()->heap.subspaceForObjectWithoutDestructor();
</del><ins>+ MarkedSpace::Subspace& subspace = m_jit.vm()->heap.subspaceForObjectOfType<ClassType>();
</ins><span class="cx"> m_jit.add32(TrustedImm32(MarkedSpace::preciseStep - 1), allocationSize);
</span><span class="cx"> MacroAssembler::Jump notSmall = m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::preciseCutoff));
</span><span class="cx"> m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::preciseStep)), scratchGPR1);
</span><span class="cx"> m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
</span><del>- m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace->preciseAllocators[0]), scratchGPR1);
</del><ins>+ m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.preciseAllocators[0]), scratchGPR1);
</ins><span class="cx">
</span><span class="cx"> MacroAssembler::Jump selectedSmallSpace = m_jit.jump();
</span><span class="cx"> notSmall.link(&m_jit);
</span><span class="cx"> slowPath.append(m_jit.branch32(MacroAssembler::AboveOrEqual, allocationSize, TrustedImm32(MarkedSpace::impreciseCutoff)));
</span><span class="cx"> m_jit.rshift32(allocationSize, TrustedImm32(getLSBSet(MarkedSpace::impreciseStep)), scratchGPR1);
</span><span class="cx"> m_jit.mul32(TrustedImm32(sizeof(MarkedAllocator)), scratchGPR1, scratchGPR1);
</span><del>- m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace->impreciseAllocators[0]), scratchGPR1);
</del><ins>+ m_jit.addPtr(MacroAssembler::TrustedImmPtr(&subspace.impreciseAllocators[0]), scratchGPR1);
</ins><span class="cx">
</span><span class="cx"> selectedSmallSpace.link(&m_jit);
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreftlFTLLowerDFGToLLVMcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -5225,14 +5225,8 @@
</span><span class="cx"> template<typename ClassType>
</span><span class="cx"> LValue allocateObject(Structure* structure, LValue butterfly, LBasicBlock slowPath)
</span><span class="cx"> {
</span><del>- MarkedAllocator* allocator;
</del><span class="cx"> size_t size = ClassType::allocationSize(0);
</span><del>- if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
- allocator = &vm().heap.allocatorForObjectWithImmortalStructureDestructor(size);
- else if (ClassType::needsDestruction)
- allocator = &vm().heap.allocatorForObjectWithNormalDestructor(size);
- else
- allocator = &vm().heap.allocatorForObjectWithoutDestructor(size);
</del><ins>+ MarkedAllocator* allocator = &vm().heap.allocatorForObjectOfType<ClassType>(size);
</ins><span class="cx"> return allocateObject(m_out.constIntPtr(allocator), structure, butterfly, slowPath);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeaph"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/Heap.h (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/Heap.h 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/heap/Heap.h 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -140,9 +140,11 @@
</span><span class="cx"> MarkedSpace::Subspace& subspaceForObjectWithoutDestructor() { return m_objectSpace.subspaceForObjectsWithoutDestructor(); }
</span><span class="cx"> MarkedSpace::Subspace& subspaceForObjectNormalDestructor() { return m_objectSpace.subspaceForObjectsWithNormalDestructor(); }
</span><span class="cx"> MarkedSpace::Subspace& subspaceForObjectsWithImmortalStructure() { return m_objectSpace.subspaceForObjectsWithImmortalStructure(); }
</span><ins>+ template<typename ClassType> MarkedSpace::Subspace& subspaceForObjectOfType();
</ins><span class="cx"> MarkedAllocator& allocatorForObjectWithoutDestructor(size_t bytes) { return m_objectSpace.allocatorFor(bytes); }
</span><span class="cx"> MarkedAllocator& allocatorForObjectWithNormalDestructor(size_t bytes) { return m_objectSpace.normalDestructorAllocatorFor(bytes); }
</span><span class="cx"> MarkedAllocator& allocatorForObjectWithImmortalStructureDestructor(size_t bytes) { return m_objectSpace.immortalStructureDestructorAllocatorFor(bytes); }
</span><ins>+ template<typename ClassType> MarkedAllocator& allocatorForObjectOfType(size_t bytes);
</ins><span class="cx"> CopiedAllocator& storageAllocator() { return m_storageSpace.allocator(); }
</span><span class="cx"> CheckedBoolean tryAllocateStorage(JSCell* intendedOwner, size_t, void**);
</span><span class="cx"> CheckedBoolean tryReallocateStorage(JSCell* intendedOwner, void**, size_t, size_t);
</span><span class="lines">@@ -259,6 +261,7 @@
</span><span class="cx"> void* allocateWithImmortalStructureDestructor(size_t); // For use with special objects whose Structures never die.
</span><span class="cx"> void* allocateWithNormalDestructor(size_t); // For use with objects that inherit directly or indirectly from JSDestructibleObject.
</span><span class="cx"> void* allocateWithoutDestructor(size_t); // For use with objects without destructors.
</span><ins>+ template<typename ClassType> void* allocateObjectOfType(size_t); // Chooses one of the methods above based on type.
</ins><span class="cx">
</span><span class="cx"> static const size_t minExtraCost = 256;
</span><span class="cx"> static const size_t maxExtraCost = 1024 * 1024;
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreheapHeapInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/heap/HeapInlines.h (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/heap/HeapInlines.h 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/heap/HeapInlines.h 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -205,6 +205,36 @@
</span><span class="cx"> return m_objectSpace.allocateWithoutDestructor(bytes);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+template<typename ClassType>
+void* Heap::allocateObjectOfType(size_t bytes)
+{
+ if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+ return allocateWithImmortalStructureDestructor(bytes);
+ if (ClassType::needsDestruction)
+ return allocateWithNormalDestructor(bytes);
+ return allocateWithoutDestructor(bytes);
+}
+
+template<typename ClassType>
+MarkedSpace::Subspace& Heap::subspaceForObjectOfType()
+{
+ if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+ return subspaceForObjectsWithImmortalStructure();
+ if (ClassType::needsDestruction)
+ return subspaceForObjectNormalDestructor();
+ return subspaceForObjectWithoutDestructor();
+}
+
+template<typename ClassType>
+MarkedAllocator& Heap::allocatorForObjectOfType(size_t bytes)
+{
+ if (ClassType::needsDestruction && ClassType::hasImmortalStructure)
+ return allocatorForObjectWithImmortalStructureDestructor(bytes);
+ if (ClassType::needsDestruction)
+ return allocatorForObjectWithNormalDestructor(bytes);
+ return allocatorForObjectWithoutDestructor(bytes);
+}
+
</ins><span class="cx"> inline CheckedBoolean Heap::tryAllocateStorage(JSCell* intendedOwner, size_t bytes, void** outPtr)
</span><span class="cx"> {
</span><span class="cx"> CheckedBoolean result = m_storageSpace.tryAllocate(bytes, outPtr);
</span></span></pre></div>
<a id="trunkSourceJavaScriptCoreruntimeJSCellInlinesh"></a>
<div class="modfile"><h4>Modified: trunk/Source/JavaScriptCore/runtime/JSCellInlines.h (181018 => 181019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/JavaScriptCore/runtime/JSCellInlines.h 2015-03-04 21:21:56 UTC (rev 181018)
+++ trunk/Source/JavaScriptCore/runtime/JSCellInlines.h 2015-03-04 21:32:27 UTC (rev 181019)
</span><span class="lines">@@ -129,13 +129,7 @@
</span><span class="cx"> {
</span><span class="cx"> ASSERT(!DisallowGC::isGCDisallowedOnCurrentThread());
</span><span class="cx"> ASSERT(size >= sizeof(T));
</span><del>- JSCell* result = 0;
- if (T::needsDestruction && T::hasImmortalStructure)
- result = static_cast<JSCell*>(heap.allocateWithImmortalStructureDestructor(size));
- else if (T::needsDestruction)
- result = static_cast<JSCell*>(heap.allocateWithNormalDestructor(size));
- else
- result = static_cast<JSCell*>(heap.allocateWithoutDestructor(size));
</del><ins>+ JSCell* result = static_cast<JSCell*>(heap.allocateObjectOfType<T>(size));
</ins><span class="cx"> #if ENABLE(GC_VALIDATION)
</span><span class="cx"> ASSERT(!heap.vm()->isInitializingObject());
</span><span class="cx"> heap.vm()->setInitializingObjectClass(T::info());
</span></span></pre>
</div>
</div>
</body>
</html>