Modified: trunk/Source/WebCore/ChangeLog (143124 => 143125)
--- trunk/Source/WebCore/ChangeLog 2013-02-17 06:01:39 UTC (rev 143124)
+++ trunk/Source/WebCore/ChangeLog 2013-02-17 08:17:42 UTC (rev 143125)
@@ -1,3 +1,59 @@
+2013-02-17 Andreas Kling <[email protected]>
+
+ Optimize GlyphPage for case where all glyphs are available in the same font.
+ <http://webkit.org/b/108835>
+ <rdar://problem/13157042>
+
+ Reviewed by Antti Koivisto.
+
+ Let GlyphPage begin optimistically assuming that all its glyphs will be represented in
+ the same SimpleFontData*. In this (very common) case, only keep a single SimpleFontData*.
+
+ If glyphs from multiple fonts are mixed in one page, an array of per-glyph SimpleFontData*
+ is allocated transparently.
+
+ This was landed before with some bogus branch prediction hints and didn't fare well on
+ page cyclers (intl2 specifically.) These have been removed this time around, and will
+ hopefully be regression-free.
+
+ 4.98 MB progression on Membuster3.
+
+ * platform/graphics/GlyphPageTreeNode.cpp:
+ (WebCore::GlyphPageTreeNode::initializePage):
+ * platform/graphics/GlyphPage.h:
+ (WebCore::GlyphPage::createUninitialized):
+ (WebCore::GlyphPage::createZeroedSystemFallbackPage):
+ (WebCore::GlyphPage::createCopiedSystemFallbackPage):
+
+ There are now three ways of constructing a GlyphPage, two of them are only used for
+ creating system fallback pages.
+
+ (WebCore::GlyphPage::setGlyphDataForIndex):
+
+ Hold off creating a SimpleFontData* array until we're sure there are two different
+ SimpleFontData* backing the glyphs in this page.
+ We don't store font data for glyph #0, instead we let the getters always return null for it.
+
+ (WebCore::GlyphPage::~GlyphPage):
+
+ Free the SimpleFontData* array if needed.
+
+ (WebCore::GlyphPage::glyphDataForCharacter):
+ (WebCore::GlyphPage::glyphDataForIndex):
+ (WebCore::GlyphPage::fontDataForCharacter):
+
+ The font data for glyph #0 is always a null pointer now.
+
+ (WebCore::GlyphPage::clearForFontData):
+
+ Updated for new storage format.
+
+ * rendering/svg/SVGTextRunRenderingContext.cpp:
+ (WebCore::SVGTextRunRenderingContext::glyphDataForCharacter):
+
+ Fix bug where non-zero glyph was temporarily associated with null font data,
+ which triggered the new assertion in setGlyphDataForIndex().
+
2013-02-16 Andreas Kling <[email protected]>
Remove multi-threading gunk from WebKit2's PluginInfoStore.
Modified: trunk/Source/WebCore/platform/graphics/GlyphPage.h (143124 => 143125)
--- trunk/Source/WebCore/platform/graphics/GlyphPage.h 2013-02-17 06:01:39 UTC (rev 143124)
+++ trunk/Source/WebCore/platform/graphics/GlyphPage.h 2013-02-17 08:17:42 UTC (rev 143125)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
+ * Copyright (C) 2006, 2007, 2008, 2013 Apple Inc. All rights reserved.
* Copyright (C) Research In Motion Limited 2011. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -33,6 +33,7 @@
#include "Glyph.h"
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
+#include <wtf/RefPtr.h>
#include <wtf/unicode/Unicode.h>
namespace WebCore {
@@ -62,24 +63,51 @@
// to be overriding the parent's node, but provide no additional information.
class GlyphPage : public RefCounted<GlyphPage> {
public:
- static PassRefPtr<GlyphPage> create(GlyphPageTreeNode* owner)
+ static PassRefPtr<GlyphPage> createUninitialized(GlyphPageTreeNode* owner)
{
- return adoptRef(new GlyphPage(owner));
+ return adoptRef(new GlyphPage(owner, false));
}
+ static PassRefPtr<GlyphPage> createZeroedSystemFallbackPage(GlyphPageTreeNode* owner)
+ {
+ return adoptRef(new GlyphPage(owner, true));
+ }
+
+ PassRefPtr<GlyphPage> createCopiedSystemFallbackPage(GlyphPageTreeNode* owner) const
+ {
+ RefPtr<GlyphPage> page = GlyphPage::createUninitialized(owner);
+ memcpy(page->m_glyphs, m_glyphs, sizeof(m_glyphs));
+ page->m_fontDataForAllGlyphs = m_fontDataForAllGlyphs;
+ if (m_perGlyphFontData) {
+ page->m_perGlyphFontData = static_cast<const SimpleFontData**>(fastMalloc(size * sizeof(SimpleFontData*)));
+ memcpy(page->m_perGlyphFontData, m_perGlyphFontData, size * sizeof(SimpleFontData*));
+ }
+ return page.release();
+ }
+
+ ~GlyphPage()
+ {
+ if (m_perGlyphFontData)
+ fastFree(m_perGlyphFontData);
+ }
+
static const size_t size = 256; // Covers Latin-1 in a single page.
unsigned indexForCharacter(UChar32 c) const { return c % size; }
GlyphData glyphDataForCharacter(UChar32 c) const
{
- unsigned index = indexForCharacter(c);
- return GlyphData(m_glyphs[index], m_glyphFontData[index]);
+ return glyphDataForIndex(indexForCharacter(c));
}
GlyphData glyphDataForIndex(unsigned index) const
{
ASSERT_WITH_SECURITY_IMPLICATION(index < size);
- return GlyphData(m_glyphs[index], m_glyphFontData[index]);
+ Glyph glyph = m_glyphs[index];
+ if (!glyph)
+ return GlyphData(0, 0);
+ if (m_perGlyphFontData)
+ return GlyphData(glyph, m_perGlyphFontData[index]);
+ return GlyphData(glyph, m_fontDataForAllGlyphs);
}
Glyph glyphAt(unsigned index) const
@@ -90,7 +118,7 @@
const SimpleFontData* fontDataForCharacter(UChar32 c) const
{
- return m_glyphFontData[indexForCharacter(c)];
+ return glyphDataForIndex(indexForCharacter(c)).fontData;
}
void setGlyphDataForCharacter(UChar32 c, Glyph g, const SimpleFontData* f)
@@ -98,36 +126,55 @@
setGlyphDataForIndex(indexForCharacter(c), g, f);
}
- void setGlyphDataForIndex(unsigned index, Glyph g, const SimpleFontData* f)
+ void setGlyphDataForIndex(unsigned index, Glyph glyph, const SimpleFontData* fontData)
{
ASSERT_WITH_SECURITY_IMPLICATION(index < size);
- m_glyphs[index] = g;
- m_glyphFontData[index] = f;
+ m_glyphs[index] = glyph;
+
+ // GlyphPage getters will always return a null SimpleFontData* for glyph #0, so don't worry about the pointer for them.
+ if (!glyph)
+ return;
+
+ // A glyph index without a font data pointer makes no sense.
+ ASSERT(fontData);
+
+ if (m_perGlyphFontData) {
+ m_perGlyphFontData[index] = fontData;
+ return;
+ }
+
+ if (!m_fontDataForAllGlyphs)
+ m_fontDataForAllGlyphs = fontData;
+
+ if (m_fontDataForAllGlyphs == fontData)
+ return;
+
+ // This GlyphPage houses glyphs from multiple fonts, transition to an array of SimpleFontData pointers.
+ const SimpleFontData* oldFontData = m_fontDataForAllGlyphs;
+ m_perGlyphFontData = static_cast<const SimpleFontData**>(fastMalloc(size * sizeof(SimpleFontData*)));
+ for (unsigned i = 0; i < size; ++i)
+ m_perGlyphFontData[i] = oldFontData;
+ m_perGlyphFontData[index] = fontData;
}
void setGlyphDataForIndex(unsigned index, const GlyphData& glyphData)
{
setGlyphDataForIndex(index, glyphData.glyph, glyphData.fontData);
}
-
- void copyFrom(const GlyphPage& other)
- {
- memcpy(m_glyphs, other.m_glyphs, sizeof(m_glyphs));
- memcpy(m_glyphFontData, other.m_glyphFontData, sizeof(m_glyphFontData));
- }
- void clear()
- {
- memset(m_glyphs, 0, sizeof(m_glyphs));
- memset(m_glyphFontData, 0, sizeof(m_glyphFontData));
- }
-
void clearForFontData(const SimpleFontData* fontData)
{
+ if (!m_perGlyphFontData) {
+ if (m_fontDataForAllGlyphs == fontData) {
+ memset(m_glyphs, 0, sizeof(m_glyphs));
+ m_fontDataForAllGlyphs = 0;
+ }
+ return;
+ }
for (size_t i = 0; i < size; ++i) {
- if (m_glyphFontData[i] == fontData) {
+ if (m_perGlyphFontData[i] == fontData) {
m_glyphs[i] = 0;
- m_glyphFontData[i] = 0;
+ m_perGlyphFontData[i] = 0;
}
}
}
@@ -138,18 +185,22 @@
bool fill(unsigned offset, unsigned length, UChar* characterBuffer, unsigned bufferLength, const SimpleFontData*);
private:
- GlyphPage(GlyphPageTreeNode* owner)
- : m_owner(owner)
+ GlyphPage(GlyphPageTreeNode* owner, bool clearGlyphs)
+ : m_fontDataForAllGlyphs(0)
+ , m_perGlyphFontData(0)
+ , m_owner(owner)
{
+ if (clearGlyphs)
+ memset(m_glyphs, 0, sizeof(m_glyphs));
}
- // Separate arrays, rather than array of GlyphData, to save space.
- Glyph m_glyphs[size];
- const SimpleFontData* m_glyphFontData[size];
+ const SimpleFontData* m_fontDataForAllGlyphs;
+ const SimpleFontData** m_perGlyphFontData;
GlyphPageTreeNode* m_owner;
+ Glyph m_glyphs[size];
};
} // namespace WebCore
-#endif // GlyphPageTreeNode_h
+#endif // GlyphPage_h
Modified: trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (143124 => 143125)
--- trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2013-02-17 06:01:39 UTC (rev 143124)
+++ trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2013-02-17 08:17:42 UTC (rev 143125)
@@ -200,9 +200,9 @@
buffer[i * 2 + 1] = U16_TRAIL(c);
}
}
-
- m_page = GlyphPage::create(this);
+ m_page = GlyphPage::createUninitialized(this);
+
// Now that we have a buffer full of characters, we want to get back an array
// of glyph indices. This part involves calling into the platform-specific
// routine of our glyph map for actually filling in the page with the glyphs.
@@ -225,7 +225,7 @@
int to = 1 + min(static_cast<int>(range.to()) - static_cast<int>(start), static_cast<int>(GlyphPage::size) - 1);
if (from < static_cast<int>(GlyphPage::size) && to > 0) {
if (haveGlyphs && !scratchPage) {
- scratchPage = GlyphPage::create(this);
+ scratchPage = GlyphPage::createUninitialized(this);
pageToFill = scratchPage.get();
}
@@ -279,7 +279,7 @@
m_page = parentPage;
} else {
// Combine the parent's glyphs and ours to form a new more complete page.
- m_page = GlyphPage::create(this);
+ m_page = GlyphPage::createUninitialized(this);
// Overlay the parent page on the fallback page. Check if the fallback font
// has added anything.
@@ -300,15 +300,14 @@
}
}
} else {
- m_page = GlyphPage::create(this);
// System fallback. Initialized with the parent's page here, as individual
// entries may use different fonts depending on character. If the Font
// ever finds it needs a glyph out of the system fallback page, it will
// ask the system for the best font to use and fill that glyph in for us.
if (parentPage)
- m_page->copyFrom(*parentPage);
+ m_page = parentPage->createCopiedSystemFallbackPage(this);
else
- m_page->clear();
+ m_page = GlyphPage::createZeroedSystemFallbackPage(this);
}
}
Modified: trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp (143124 => 143125)
--- trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp 2013-02-17 06:01:39 UTC (rev 143124)
+++ trunk/Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp 2013-02-17 08:17:42 UTC (rev 143125)
@@ -230,7 +230,7 @@
// No suitable glyph found that is compatible with the requirments (same language, arabic-form, orientation etc.)
// Even though our GlyphPage contains an entry for eg. glyph "a", it's not compatible. So we have to temporarily
// remove the glyph data information from the GlyphPage, and retry the lookup, which handles font fallbacks correctly.
- page->setGlyphDataForCharacter(character, glyphData.glyph, 0);
+ page->setGlyphDataForCharacter(character, 0, 0);
// Assure that the font fallback glyph selection worked, aka. the fallbackGlyphData font data is not the same as before.
GlyphData fallbackGlyphData = font.glyphDataForCharacter(character, mirror);
