Skip to content

Commit

Permalink
[WebCore] Optimize Font::applyTransforms
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270406
rdar://123961009

Reviewed by Chris Dumez.

Font::applyTransforms is very slow. While the most of time is used in CoreText, the other part is also using much time!
This patch optimizes it.

1. We add Vector::insertFill function to insert one-item-filling into Vector. GlyphBuffer is
   doing this in a very inefficient way right now: allocating filled Vector and using insertVector.
2. Add size parameter to upconvertedCharacters and use 256 for static Vector size in Font::applyTransforms,
   to avoid unnecessary allocations.
3. LocaleCocoa::canonicalLanguageIdentifierFromString should return RetainPtr<CFStringRef>. We found that
   we are super repeatedly creating CFString when locale is specified because canonicalLanguageIdentifierFromString
   returns AtomString and we convert it to CFString. And this is very slow. Because canonicalLanguageIdentifierFromString
   is only used in this place, we should just return RetainPtr<CFStringRef>. Also we optimized the caching mechanism
   in canonicalLanguageIdentifierFromString to cache the one item out of HashMap since this one-item cache can cover almost
   all cases.

* Source/WTF/wtf/Vector.h:
(WTF::Malloc>::insertFill):
* Source/WTF/wtf/text/StringView.h:
(WTF::StringView::upconvertedCharacters const):
(WTF::StringView::UpconvertedCharacters<N>::UpconvertedCharacters):
(WTF::StringView::UpconvertedCharacters::UpconvertedCharacters): Deleted.
* Source/WebCore/editing/TextIterator.cpp:
* Source/WebCore/platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::makeHole):
* Source/WebCore/platform/graphics/coretext/FontCoreText.cpp:
(WebCore::Font::applyTransforms const):
* Source/WebCore/platform/text/cocoa/LocaleCocoa.h:
* Source/WebCore/platform/text/cocoa/LocaleCocoa.mm:
(WebCore::localeCache):
(WebCore::LocaleCocoa::canonicalLanguageIdentifierFromString):
(WebCore::LocaleCocoa::releaseMemory):
(WebCore::canonicalLocaleMap): Deleted.

Canonical link: https://commits.webkit.org/275676@main
  • Loading branch information
Constellation committed Mar 5, 2024
1 parent bc31443 commit 4e349ca
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 29 deletions.
21 changes: 20 additions & 1 deletion Source/WTF/wtf/Vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ class Vector : private VectorBuffer<T, inlineCapacity, Malloc> {
void appendUsingFunctor(size_t, const Functor&);

void insert(size_t position, value_type&& value) { insert<value_type>(position, std::forward<value_type>(value)); }
void insertFill(size_t position, const T& value, size_t dataSize);
template<typename U> void insert(size_t position, const U*, size_t);
template<typename U> void insert(size_t position, U&&);
template<typename U, size_t c, typename OH, size_t m, typename M> void insertVector(size_t position, const Vector<U, c, OH, m, M>&);
Expand Down Expand Up @@ -1612,7 +1613,7 @@ void Vector<T, inlineCapacity, OverflowHandler, minCapacity, Malloc>::insert(siz
VectorCopier<std::is_trivial<T>::value, U>::uninitializedCopy(data, std::addressof(data[dataSize]), spot);
m_size = newSize;
}

template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity, typename Malloc>
template<typename U>
inline void Vector<T, inlineCapacity, OverflowHandler, minCapacity, Malloc>::insert(size_t position, U&& value)
Expand All @@ -1633,6 +1634,24 @@ inline void Vector<T, inlineCapacity, OverflowHandler, minCapacity, Malloc>::ins
++m_size;
}

template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity, typename Malloc>
void Vector<T, inlineCapacity, OverflowHandler, minCapacity, Malloc>::insertFill(size_t position, const T& data, size_t dataSize)
{
ASSERT_WITH_SECURITY_IMPLICATION(position <= size());
size_t newSize = m_size + dataSize;
if (newSize > capacity()) {
expandCapacity<FailureAction::Crash>(newSize);
ASSERT(begin());
}
if (newSize < m_size)
CRASH();
asanBufferSizeWillChangeTo(newSize);
T* spot = begin() + position;
TypeOperations::moveOverlapping(spot, end(), spot + dataSize);
TypeOperations::uninitializedFill(spot, spot + dataSize, data);
m_size = newSize;
}

template<typename T, size_t inlineCapacity, typename OverflowHandler, size_t minCapacity, typename Malloc>
template<typename U, size_t c, typename OH, size_t m, typename M>
inline void Vector<T, inlineCapacity, OverflowHandler, minCapacity, Malloc>::insertVector(size_t position, const Vector<U, c, OH, m, M>& val)
Expand Down
23 changes: 15 additions & 8 deletions Source/WTF/wtf/text/StringView.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,12 @@ class StringView final {
template<typename Func>
Expected<std::invoke_result_t<Func, std::span<const char>>, UTF8ConversionError> tryGetUTF8(const Func&, ConversionMode = LenientConversion) const;

class UpconvertedCharacters;
UpconvertedCharacters upconvertedCharacters() const;
template<size_t N>
class UpconvertedCharactersWithSize;
using UpconvertedCharacters = UpconvertedCharactersWithSize<32>;

template<size_t N = 32>
UpconvertedCharactersWithSize<N> upconvertedCharacters() const;

template<typename CharacterType> void getCharacters(CharacterType*) const;
template<typename CharacterType> void getCharacters8(CharacterType*) const;
Expand Down Expand Up @@ -511,20 +515,22 @@ inline bool StringView::containsOnlyASCII() const
return charactersAreAllASCII(characters16(), length());
}

class StringView::UpconvertedCharacters {
template<size_t N>
class StringView::UpconvertedCharactersWithSize {
WTF_MAKE_FAST_ALLOCATED;
public:
explicit UpconvertedCharacters(StringView);
explicit UpconvertedCharactersWithSize(StringView);
operator const UChar*() const { return m_characters; }
const UChar* get() const { return m_characters; }
private:
Vector<UChar, 32> m_upconvertedCharacters;
Vector<UChar, N> m_upconvertedCharacters;
const UChar* m_characters;
};

inline StringView::UpconvertedCharacters StringView::upconvertedCharacters() const
template<size_t N>
inline StringView::UpconvertedCharactersWithSize<N> StringView::upconvertedCharacters() const
{
return UpconvertedCharacters(*this);
return UpconvertedCharactersWithSize<N>(*this);
}

inline bool StringView::isNull() const
Expand Down Expand Up @@ -631,7 +637,8 @@ template<typename CharacterType> inline void StringView::getCharacters(Character
getCharacters16(destination);
}

inline StringView::UpconvertedCharacters::UpconvertedCharacters(StringView string)
template<size_t N>
inline StringView::UpconvertedCharactersWithSize<N>::UpconvertedCharactersWithSize(StringView string)
{
if (!string.is8Bit()) {
m_characters = string.characters16();
Expand Down
6 changes: 3 additions & 3 deletions Source/WebCore/editing/cocoa/DataDetection.mm
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,8 @@ static void buildQuery(DDScanQueryRef scanQuery, const SimpleRange& contextRange
continue;
}
// Test for white space nodes, we're coalescing them.
auto currentTextUpconvertedCharacters = currentText.upconvertedCharacters();
auto currentCharPtr = currentTextUpconvertedCharacters.get();
auto currentTextUpconvertedCharactersWithSize = currentText.upconvertedCharacters();
auto currentCharPtr = currentTextUpconvertedCharactersWithSize.get();

bool containsOnlyWhiteSpace = true;
bool hasTab = false;
Expand Down Expand Up @@ -415,7 +415,7 @@ static void buildQuery(DDScanQueryRef scanQuery, const SimpleRange& contextRange
continue;
}

RetainPtr currentTextCFString = adoptCF(CFStringCreateWithCharacters(kCFAllocatorDefault, reinterpret_cast<const UniChar*>(currentTextUpconvertedCharacters.get()), currentTextLength));
RetainPtr currentTextCFString = adoptCF(CFStringCreateWithCharacters(kCFAllocatorDefault, reinterpret_cast<const UniChar*>(currentTextUpconvertedCharactersWithSize.get()), currentTextLength));

PAL::softLink_DataDetectorsCore_DDScanQueryAddTextFragment(scanQuery, currentTextCFString.get(), CFRangeMake(0, currentTextLength), (void *)iteratorCount, (DDTextFragmentMode)0, DDTextCoalescingTypeNone);
}
Expand Down
10 changes: 5 additions & 5 deletions Source/WebCore/platform/graphics/GlyphBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,11 @@ class GlyphBuffer : public CanMakeCheckedPtr {
{
ASSERT(location <= size());

m_fonts.insertVector(location, Vector<const Font*>(length, font));
m_glyphs.insertVector(location, Vector<GlyphBufferGlyph>(length, std::numeric_limits<GlyphBufferGlyph>::max()));
m_advances.insertVector(location, Vector<GlyphBufferAdvance>(length, makeGlyphBufferAdvance()));
m_origins.insertVector(location, Vector<GlyphBufferOrigin>(length, makeGlyphBufferOrigin()));
m_offsetsInString.insertVector(location, Vector<GlyphBufferStringOffset>(length, 0));
m_fonts.insertFill(location, font, length);
m_glyphs.insertFill(location, std::numeric_limits<GlyphBufferGlyph>::max(), length);
m_advances.insertFill(location, makeGlyphBufferAdvance(), length);
m_origins.insertFill(location, makeGlyphBufferOrigin(), length);
m_offsetsInString.insertFill(location, 0, length);
}

void reverse(unsigned from, unsigned length)
Expand Down
5 changes: 3 additions & 2 deletions Source/WebCore/platform/graphics/coretext/FontCoreText.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -596,8 +596,9 @@ GlyphBufferAdvance Font::applyTransforms(GlyphBuffer& glyphBuffer, unsigned begi
};

auto substring = text.substring(beginningStringIndex);
auto upconvertedCharacters = substring.upconvertedCharacters();
auto localeString = locale.isNull() ? nullptr : LocaleCocoa::canonicalLanguageIdentifierFromString(locale).string().createCFString();
constexpr unsigned bufferSize = 256;
auto upconvertedCharacters = substring.upconvertedCharacters<bufferSize>();
auto localeString = locale.isNull() ? nullptr : LocaleCocoa::canonicalLanguageIdentifierFromString(locale);
auto numberOfInputGlyphs = glyphBuffer.size() - beginningGlyphIndex;
// FIXME: Enable kerning for single glyphs when rdar://82195405 is fixed
CTFontShapeOptions options = kCTFontShapeWithClusterComposition
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/text/cocoa/LocaleCocoa.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class LocaleCocoa final : public Locale {
const Vector<String>& timeAMPMLabels() override;
#endif

static AtomString canonicalLanguageIdentifierFromString(const AtomString&);
static RetainPtr<CFStringRef> canonicalLanguageIdentifierFromString(const AtomString&);
static void releaseMemory();

private:
Expand Down
35 changes: 26 additions & 9 deletions Source/WebCore/platform/text/cocoa/LocaleCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -245,26 +245,43 @@

#endif

using CanonicalLocaleMap = HashMap<AtomString, AtomString>;
using CanonicalLocaleMap = HashMap<AtomString, RetainPtr<CFStringRef>>;

static CanonicalLocaleMap& canonicalLocaleMap()
struct LocaleCache {
AtomString m_key;
RetainPtr<CFStringRef> m_value;
CanonicalLocaleMap m_map;
};


static LocaleCache& localeCache()
{
static NeverDestroyed<CanonicalLocaleMap> canonicalLocaleMap;
return canonicalLocaleMap.get();
static MainThreadNeverDestroyed<LocaleCache> localeCache;
return localeCache.get();
}

AtomString LocaleCocoa::canonicalLanguageIdentifierFromString(const AtomString& string)
RetainPtr<CFStringRef> LocaleCocoa::canonicalLanguageIdentifierFromString(const AtomString& string)
{
if (string.isEmpty())
return string;
return canonicalLocaleMap().ensure(string, [&] {
return [NSLocale canonicalLanguageIdentifierFromString:string];
return CFSTR("");

auto& cache = localeCache();
if (cache.m_key == string)
return cache.m_value;
auto result = cache.m_map.ensure(string, [&] {
return (__bridge CFStringRef)[NSLocale canonicalLanguageIdentifierFromString:string];
}).iterator->value;
cache.m_key = string;
cache.m_value = result;
return result;
}

void LocaleCocoa::releaseMemory()
{
canonicalLocaleMap().clear();
auto& cache = localeCache();
cache.m_key = { };
cache.m_value = { };
cache.m_map.clear();
}

void LocaleCocoa::initializeLocaleData()
Expand Down
22 changes: 22 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/Vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2086,4 +2086,26 @@ TEST(WTF_Vector, FlatMapInnerStruct)
EXPECT_EQ(4, mapped[3]);
}

TEST(WTF_Vector, InsertFill)
{
Vector<unsigned> vector;

for (size_t i = 0; i < 100; ++i)
vector.append(i);

EXPECT_EQ(vector.size(), 100U);

vector.insertFill(50, 0xffff, 100);
EXPECT_EQ(vector.size(), 200U);

for (size_t i = 0; i < 50; ++i)
EXPECT_EQ(vector[i], i);

for (size_t i = 0; i < 100; ++i)
EXPECT_EQ(vector[i + 50], 0xffffU);

for (size_t i = 0; i < 50; ++i)
EXPECT_EQ(vector[i + 150], i + 50U);
}

} // namespace TestWebKitAPI

0 comments on commit 4e349ca

Please sign in to comment.