Modified: trunk/Source/_javascript_Core/ChangeLog (143957 => 143958)
--- trunk/Source/_javascript_Core/ChangeLog 2013-02-25 20:27:41 UTC (rev 143957)
+++ trunk/Source/_javascript_Core/ChangeLog 2013-02-25 20:48:33 UTC (rev 143958)
@@ -1,3 +1,53 @@
+2013-02-24 Filip Pizlo <[email protected]>
+
+ DFG::Edge should have more bits for UseKind, and DFG::Allocator should be simpler
+ https://bugs.webkit.org/show_bug.cgi?id=110722
+
+ Reviewed by Oliver Hunt.
+
+ This rolls out the DFG::Allocator part of http://trac.webkit.org/changeset/143654,
+ and changes Edge to have more room for UseKinds and possibly other things.
+
+ This is performance-neutral on both 32-bit and 64-bit. It reduces the size of
+ DFG::Node on 64-bit (by virtue of getting rid of the 16-byte alignment of Node)
+ and increases it slightly on 32-bit (by 4 bytes total - 16-byte alignment led to
+ 80 bytes, but the base size of Node plus the 12 bytes of new m_encodedWords in
+ Edge gets 84 bytes). But, it will mean that we don't have to increase Node by
+ another 16 bytes if we ever want to add more UseKinds or other things to Edge.
+
+ * dfg/DFGAllocator.h:
+ (DFG):
+ (Allocator):
+ (JSC::DFG::Allocator::Region::headerSize):
+ (JSC::DFG::Allocator::Region::numberOfThingsPerRegion):
+ (JSC::DFG::Allocator::Region::data):
+ (JSC::DFG::Allocator::Region::isInThisRegion):
+ (JSC::DFG::::Allocator):
+ (JSC::DFG::::~Allocator):
+ (JSC::DFG::::allocate):
+ (JSC::DFG::::free):
+ (JSC::DFG::::freeAll):
+ (JSC::DFG::::reset):
+ (JSC::DFG::::indexOf):
+ (JSC::DFG::::allocatorOf):
+ (JSC::DFG::::bumpAllocate):
+ (JSC::DFG::::freeListAllocate):
+ (JSC::DFG::::allocateSlow):
+ (JSC::DFG::::freeRegionsStartingAt):
+ (JSC::DFG::::startBumpingIn):
+ * dfg/DFGEdge.h:
+ (JSC::DFG::Edge::Edge):
+ (Edge):
+ (JSC::DFG::Edge::node):
+ (JSC::DFG::Edge::setNode):
+ (JSC::DFG::Edge::useKindUnchecked):
+ (JSC::DFG::Edge::setUseKind):
+ (JSC::DFG::Edge::operator==):
+ (JSC::DFG::Edge::operator!=):
+ (JSC::DFG::Edge::makeWord):
+ * dfg/DFGNodeAllocator.h:
+ (DFG):
+
2013-02-22 Filip Pizlo <[email protected]>
The DFG special case checks for isCreatedThisArgument are fragile
Modified: trunk/Source/_javascript_Core/dfg/DFGAllocator.h (143957 => 143958)
--- trunk/Source/_javascript_Core/dfg/DFGAllocator.h 2013-02-25 20:27:41 UTC (rev 143957)
+++ trunk/Source/_javascript_Core/dfg/DFGAllocator.h 2013-02-25 20:48:33 UTC (rev 143958)
@@ -43,11 +43,7 @@
// anything. You can just call freeAll() instead.
// - You call free() on all T's that you allocated, and never use freeAll().
-// forcedCellSize: set this to 0 if you're happy with the default alignment, or set it
-// to a non-zero value if you want to forcibly align the allocations to a certain
-// value. When you do this, Allocator will ASSERT that forcedCellAlignment >= sizeof(T).
-
-template<typename T, unsigned forcedCellAlignment>
+template<typename T>
class Allocator {
public:
Allocator();
@@ -67,24 +63,13 @@
void* bumpAllocate();
void* freeListAllocate();
void* allocateSlow();
-
- static size_t cellSize()
- {
- if (!forcedCellAlignment)
- return sizeof(T);
-
- ASSERT(forcedCellAlignment >= sizeof(T));
- return forcedCellAlignment;
- }
-
+
struct Region {
static size_t size() { return 64 * KB; }
- static size_t headerSize() { return std::max(sizeof(Region), cellSize()); }
- static unsigned numberOfThingsPerRegion() { return (size() - headerSize()) / cellSize(); }
- static size_t payloadSize() { return numberOfThingsPerRegion() * cellSize(); }
- char* payloadBegin() { return bitwise_cast<char*>(this) + headerSize(); }
- char* payloadEnd() { return payloadBegin() + payloadSize(); }
- bool isInThisRegion(const T* pointer) { return static_cast<size_t>(bitwise_cast<char*>(pointer) - payloadBegin()) < payloadSize(); }
+ static size_t headerSize() { return std::max(sizeof(Region), sizeof(T)); }
+ static unsigned numberOfThingsPerRegion() { return (size() - headerSize()) / sizeof(T); }
+ T* data() { return bitwise_cast<T*>(bitwise_cast<char*>(this) + headerSize()); }
+ bool isInThisRegion(const T* pointer) { return static_cast<unsigned>(pointer - data()) < numberOfThingsPerRegion(); }
static Region* regionFor(const T* pointer) { return bitwise_cast<Region*>(bitwise_cast<uintptr_t>(pointer) & ~(size() - 1)); }
PageAllocationAligned m_allocation;
@@ -97,26 +82,26 @@
Region* m_regionHead;
void** m_freeListHead;
- char* m_bumpEnd;
- size_t m_bumpRemaining;
+ T* m_bumpEnd;
+ unsigned m_bumpRemaining;
};
-template<typename T, unsigned forcedCellAlignment>
-inline Allocator<T, forcedCellAlignment>::Allocator()
+template<typename T>
+inline Allocator<T>::Allocator()
: m_regionHead(0)
, m_freeListHead(0)
, m_bumpRemaining(0)
{
}
-template<typename T, unsigned forcedCellAlignment>
-inline Allocator<T, forcedCellAlignment>::~Allocator()
+template<typename T>
+inline Allocator<T>::~Allocator()
{
reset();
}
-template<typename T, unsigned forcedCellAlignment>
-ALWAYS_INLINE void* Allocator<T, forcedCellAlignment>::allocate()
+template<typename T>
+ALWAYS_INLINE void* Allocator<T>::allocate()
{
void* result = bumpAllocate();
if (LIKELY(!!result))
@@ -124,8 +109,8 @@
return freeListAllocate();
}
-template<typename T, unsigned forcedCellAlignment>
-void Allocator<T, forcedCellAlignment>::free(T* object)
+template<typename T>
+void Allocator<T>::free(T* object)
{
object->~T();
@@ -134,8 +119,8 @@
m_freeListHead = cell;
}
-template<typename T, unsigned forcedCellAlignment>
-void Allocator<T, forcedCellAlignment>::freeAll()
+template<typename T>
+void Allocator<T>::freeAll()
{
if (!m_regionHead) {
ASSERT(!m_bumpRemaining);
@@ -157,8 +142,8 @@
startBumpingIn(m_regionHead);
}
-template<typename T, unsigned forcedCellAlignment>
-void Allocator<T, forcedCellAlignment>::reset()
+template<typename T>
+void Allocator<T>::reset()
{
freeRegionsStartingAt(m_regionHead);
@@ -167,38 +152,38 @@
m_bumpRemaining = 0;
}
-template<typename T, unsigned forcedCellAlignment>
-unsigned Allocator<T, forcedCellAlignment>::indexOf(const T* object)
+template<typename T>
+unsigned Allocator<T>::indexOf(const T* object)
{
unsigned baseIndex = 0;
for (Region* region = m_regionHead; region; region = region->m_next) {
if (region->isInThisRegion(object))
- return baseIndex + (bitwise_cast<char*>(object) - region->payloadBegin()) / cellSize();
+ return baseIndex + (object - region->data());
baseIndex += Region::numberOfThingsPerRegion();
}
CRASH();
return 0;
}
-template<typename T, unsigned forcedCellAlignment>
-Allocator<T, forcedCellAlignment>* Allocator<T, forcedCellAlignment>::allocatorOf(const T* object)
+template<typename T>
+Allocator<T>* Allocator<T>::allocatorOf(const T* object)
{
return Region::regionFor(object)->m_allocator;
}
-template<typename T, unsigned forcedCellAlignment>
-ALWAYS_INLINE void* Allocator<T, forcedCellAlignment>::bumpAllocate()
+template<typename T>
+ALWAYS_INLINE void* Allocator<T>::bumpAllocate()
{
- if (size_t remaining = m_bumpRemaining) {
- remaining -= cellSize();
+ if (unsigned remaining = m_bumpRemaining) {
+ remaining--;
m_bumpRemaining = remaining;
- return m_bumpEnd - (remaining + cellSize());
+ return m_bumpEnd - (remaining + 1);
}
return 0;
}
-template<typename T, unsigned forcedCellAlignment>
-void* Allocator<T, forcedCellAlignment>::freeListAllocate()
+template<typename T>
+void* Allocator<T>::freeListAllocate()
{
void** result = m_freeListHead;
if (UNLIKELY(!result))
@@ -207,8 +192,8 @@
return result;
}
-template<typename T, unsigned forcedCellAlignment>
-void* Allocator<T, forcedCellAlignment>::allocateSlow()
+template<typename T>
+void* Allocator<T>::allocateSlow()
{
ASSERT(!m_freeListHead);
ASSERT(!m_bumpRemaining);
@@ -231,8 +216,8 @@
return result;
}
-template<typename T, unsigned forcedCellAlignment>
-void Allocator<T, forcedCellAlignment>::freeRegionsStartingAt(typename Allocator<T, forcedCellAlignment>::Region* region)
+template<typename T>
+void Allocator<T>::freeRegionsStartingAt(typename Allocator<T>::Region* region)
{
while (region) {
Region* nextRegion = region->m_next;
@@ -241,11 +226,11 @@
}
}
-template<typename T, unsigned forcedCellAlignment>
-void Allocator<T, forcedCellAlignment>::startBumpingIn(typename Allocator<T, forcedCellAlignment>::Region* region)
+template<typename T>
+void Allocator<T>::startBumpingIn(typename Allocator<T>::Region* region)
{
- m_bumpEnd = region->payloadEnd();
- m_bumpRemaining = Region::payloadSize();
+ m_bumpEnd = region->data() + Region::numberOfThingsPerRegion();
+ m_bumpRemaining = Region::numberOfThingsPerRegion();
}
} } // namespace JSC::DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGEdge.h (143957 => 143958)
--- trunk/Source/_javascript_Core/dfg/DFGEdge.h 2013-02-25 20:27:41 UTC (rev 143957)
+++ trunk/Source/_javascript_Core/dfg/DFGEdge.h 2013-02-25 20:48:33 UTC (rev 143958)
@@ -40,34 +40,62 @@
class Edge {
public:
Edge()
+#if USE(JSVALUE64)
: m_encodedWord(makeWord(0, UntypedUse))
+#else
+ : m_node(0)
+ , m_encodedWord(UntypedUse)
+#endif
{
}
explicit Edge(Node* node)
+#if USE(JSVALUE64)
: m_encodedWord(makeWord(node, UntypedUse))
+#else
+ : m_node(node)
+ , m_encodedWord(UntypedUse)
+#endif
{
}
Edge(Node* node, UseKind useKind)
+#if USE(JSVALUE64)
: m_encodedWord(makeWord(node, useKind))
+#else
+ : m_node(node)
+ , m_encodedWord(useKind)
+#endif
{
}
- Node* node() const { return bitwise_cast<Node*>(m_encodedWord & ~((1 << shift()) - 1)); }
+#if USE(JSVALUE64)
+ Node* node() const { return bitwise_cast<Node*>(m_encodedWord >> shift()); }
+#else
+ Node* node() const { return m_node; }
+#endif
+
Node& operator*() const { return *node(); }
Node* operator->() const { return node(); }
void setNode(Node* node)
{
+#if USE(JSVALUE64)
m_encodedWord = makeWord(node, useKind());
+#else
+ m_node = node;
+#endif
}
UseKind useKindUnchecked() const
{
+#if USE(JSVALUE64)
unsigned masked = m_encodedWord & (((1 << shift()) - 1));
ASSERT(masked < LastUseKind);
UseKind result = static_cast<UseKind>(masked);
+#else
+ UseKind result = static_cast<UseKind>(m_encodedWord);
+#endif
ASSERT(node() || result == UntypedUse);
return result;
}
@@ -79,7 +107,11 @@
void setUseKind(UseKind useKind)
{
ASSERT(node());
+#if USE(JSVALUE64)
m_encodedWord = makeWord(node(), useKind);
+#else
+ m_encodedWord = useKind;
+#endif
}
bool isSet() const { return !!node(); }
@@ -91,11 +123,15 @@
bool operator==(Edge other) const
{
+#if USE(JSVALUE64)
return m_encodedWord == other.m_encodedWord;
+#else
+ return m_node == other.m_node && m_encodedWord == other.m_encodedWord;
+#endif
}
bool operator!=(Edge other) const
{
- return m_encodedWord != other.m_encodedWord;
+ return !(*this == other);
}
void dump(PrintStream&) const;
@@ -103,17 +139,25 @@
private:
friend class AdjacencyList;
+#if USE(JSVALUE64)
static uint32_t shift() { return 4; }
static uintptr_t makeWord(Node* node, UseKind useKind)
{
- uintptr_t value = bitwise_cast<uintptr_t>(node);
- ASSERT(!(value & ((1 << shift()) - 1)));
+ ASSERT(sizeof(node) == 8);
+ uintptr_t shiftedValue = bitwise_cast<uintptr_t>(node) << shift();
+ ASSERT((shiftedValue >> shift()) == bitwise_cast<uintptr_t>(node));
ASSERT(useKind >= 0 && useKind < LastUseKind);
ASSERT(LastUseKind <= (1 << shift()));
- return value | static_cast<uintptr_t>(useKind);
+ return shiftedValue | static_cast<uintptr_t>(useKind);
}
+#else
+ Node* m_node;
+#endif
+ // On 64-bit this holds both the pointer and the use kind, while on 32-bit
+ // this just holds the use kind. In both cases this may be hijacked by
+ // AdjacencyList for storing firstChild and numChildren.
uintptr_t m_encodedWord;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeAllocator.h (143957 => 143958)
--- trunk/Source/_javascript_Core/dfg/DFGNodeAllocator.h 2013-02-25 20:27:41 UTC (rev 143957)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeAllocator.h 2013-02-25 20:48:33 UTC (rev 143958)
@@ -35,16 +35,7 @@
namespace JSC { namespace DFG {
-// The second template argument to Allocator is the expected size of Node rounded up to
-// 16 bytes. This is baked in to give us assertion coverage for when Node increases in
-// size. We don't want its size to increase for no good reason. The multiple-of-16
-// property is asserted by DFG::Edge, which expects to never see any of the low 4 bits
-// of a Node* being non-zero.
-#if USE(JSVALUE64)
-typedef Allocator<Node, 112> NodeAllocator;
-#else
-typedef Allocator<Node, 80> NodeAllocator;
-#endif
+typedef Allocator<Node> NodeAllocator;
} } // namespace JSC::DFG
