Title: [132970] trunk/Source/WebCore
- Revision
- 132970
- Author
- [email protected]
- Date
- 2012-10-30 18:10:11 -0700 (Tue, 30 Oct 2012)
Log Message
Improve performance of MaskPtr.
https://bugs.webkit.org/show_bug.cgi?id=100790
Reviewed by Eric Seidel.
Calculate the mask once, and store it as a fast-access member variable.
Also avoid unneccessary integer width expansion in index calculation.
Parser/tiny-innerHTML.html has a high stddev.
Best result I've seen pre-patch is 5.70 runs/s.
Best result I've seen post-patch is 5.72 runs/s, but this is not statistically significant.
MaskPtr is still showing as ~2% in the profile, so we're not sure we trust the profile symbolization at this time.
MaskPtr is now reduced to a single inline instruction (was: 4) so this seems like a strict improvement worth landing.
* rendering/RenderArena.cpp:
(MaskPtr): Use a passed-in mask for the mask operation.
(WebCore::RenderArena::RenderArena): Calculate the mask and store it.
(WebCore::RenderArena::allocate):
(WebCore::RenderArena::free): Use stored mask and avoid unneccessary casts.
* rendering/RenderArena.h:
(RenderArena): Store the freelist mask as a member variable.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (132969 => 132970)
--- trunk/Source/WebCore/ChangeLog 2012-10-31 00:45:16 UTC (rev 132969)
+++ trunk/Source/WebCore/ChangeLog 2012-10-31 01:10:11 UTC (rev 132970)
@@ -1,3 +1,26 @@
+2012-10-30 Chris Evans <[email protected]>
+
+ Improve performance of MaskPtr.
+ https://bugs.webkit.org/show_bug.cgi?id=100790
+
+ Reviewed by Eric Seidel.
+
+ Calculate the mask once, and store it as a fast-access member variable.
+ Also avoid unneccessary integer width expansion in index calculation.
+ Parser/tiny-innerHTML.html has a high stddev.
+ Best result I've seen pre-patch is 5.70 runs/s.
+ Best result I've seen post-patch is 5.72 runs/s, but this is not statistically significant.
+ MaskPtr is still showing as ~2% in the profile, so we're not sure we trust the profile symbolization at this time.
+ MaskPtr is now reduced to a single inline instruction (was: 4) so this seems like a strict improvement worth landing.
+
+ * rendering/RenderArena.cpp:
+ (MaskPtr): Use a passed-in mask for the mask operation.
+ (WebCore::RenderArena::RenderArena): Calculate the mask and store it.
+ (WebCore::RenderArena::allocate):
+ (WebCore::RenderArena::free): Use stored mask and avoid unneccessary casts.
+ * rendering/RenderArena.h:
+ (RenderArena): Store the freelist mask as a member variable.
+
2012-10-30 Kenichi Ishibashi <[email protected]>
local(Helvetica) in src descriptor prevent fallback
Modified: trunk/Source/WebCore/rendering/RenderArena.cpp (132969 => 132970)
--- trunk/Source/WebCore/rendering/RenderArena.cpp 2012-10-31 00:45:16 UTC (rev 132969)
+++ trunk/Source/WebCore/rendering/RenderArena.cpp 2012-10-31 01:10:11 UTC (rev 132970)
@@ -43,18 +43,8 @@
#define ROUNDUP(x, y) ((((x)+((y)-1))/(y))*(y))
#ifdef NDEBUG
-// Mask freelist pointers to detect corruption and prevent freelist spraying.
-// We use an arbitray function and rely on ASLR to randomize it.
-// The first value in RenderObject (or any class) is a vtable pointer, which always
-// overlaps with the next pointer. This change guarantees that the masked vtable/next
-// pointer will never point to valid memory. So, we should immediately crash on the
-// first invalid vtable access for a stale RenderObject pointer.
-// See http://download.crowdstrike.com/papers/hes-exploiting-a-coalmine.pdf.
-static void* MaskPtr(void* p)
+static void* MaskPtr(void* p, uintptr_t mask)
{
- // The bottom bits are predictable because the binary is loaded on a boundary.
- // This just shifts most of those predictable bits out.
- const uintptr_t mask = ~(reinterpret_cast<uintptr_t>(WTF::fastMalloc) >> 13);
return reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(p) ^ mask);
}
#endif
@@ -85,6 +75,19 @@
// Zero out the recyclers array
memset(m_recyclers, 0, sizeof(m_recyclers));
+
+ // Mask freelist pointers to detect corruption and stop freelist spraying.
+ // We use an arbitray function and rely on ASLR to randomize it.
+ // The first value in RenderObject (or any class) is a vtable pointer, which
+ // always overlaps with the next pointer. This change guarantees that the
+ // masked vtable/next pointer will never point to valid memory. So, we
+ // should immediately crash on the first invalid vtable access for a stale
+ // RenderObject pointer.
+ // See http://download.crowdstrike.com/papers/hes-exploiting-a-coalmine.pdf.
+
+ // The bottom bits are predictable because the binary is loaded on a
+ // boundary. This just shifts most of those predictable bits out.
+ m_mask = ~(reinterpret_cast<uintptr_t>(WTF::fastMalloc) >> 13);
}
RenderArena::~RenderArena()
@@ -115,12 +118,12 @@
// Check recyclers first
if (size < gMaxRecycledSize) {
- const int index = size >> 2;
+ const size_t index = size >> 2;
result = m_recyclers[index];
if (result) {
// Need to move to the next object
- void* next = MaskPtr(*((void**)result));
+ void* next = MaskPtr(*((void**)result), m_mask);
m_recyclers[index] = next;
}
}
@@ -157,10 +160,10 @@
// See if it's a size that we recycle
if (size < gMaxRecycledSize) {
- const int index = size >> 2;
+ const size_t index = size >> 2;
void* currentTop = m_recyclers[index];
m_recyclers[index] = ptr;
- *((void**)ptr) = MaskPtr(currentTop);
+ *((void**)ptr) = MaskPtr(currentTop, m_mask);
}
#endif
}
Modified: trunk/Source/WebCore/rendering/RenderArena.h (132969 => 132970)
--- trunk/Source/WebCore/rendering/RenderArena.h 2012-10-31 00:45:16 UTC (rev 132969)
+++ trunk/Source/WebCore/rendering/RenderArena.h 2012-10-31 01:10:11 UTC (rev 132970)
@@ -60,6 +60,8 @@
// Underlying arena pool
ArenaPool m_pool;
+ // The mask used to secure the recycled freelist pointers.
+ uintptr_t m_mask;
// The recycler array is sparse with the indices being multiples of 4,
// i.e., 0, 4, 8, 12, 16, 20, ...
void* m_recyclers[gMaxRecycledSize >> 2];
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
