Changeset 243118 in webkit


Ignore:
Timestamp:
Mar 18, 2019 5:27:10 PM (5 years ago)
Author:
Darin Adler
Message:

Cut down on use of StringBuffer, possibly leading toward removing it entirely
​https://bugs.webkit.org/show_bug.cgi?id=195870

Reviewed by Daniel Bates.

Source/WebCore:

  • dom/Document.cpp:

(WebCore::canonicalizedTitle): Fixed all the problems mentioned in "FIXME".
Made this a single function rather than a function template. Switch to
StringBuilder instead of StringBuffer. Return the original string if the
canonicalize operation doesn't change anything.
(WebCore::Document::updateTitle): Updated for the change above.

  • platform/Length.cpp:

(WebCore::newCoordsArray): Use createUninitialized instead of StringBuffer.
Also got rid of unneeded use of upconvertedCharacters on a temporary string
that we explicitly created with 16-bit characters. The performance of this
function could be considerably simplified by not copying the original string
at all, but didn't do that at this time.

  • platform/text/TextCodecUTF16.cpp:

(WebCore::TextCodecUTF16::decode): Use createUninitialized instead of
StringBuffer. Also renamed numChars to numCodeUnits to both switch to complete
words and to be slightly more accurate.

  • rendering/RenderText.cpp:

(WebCore::convertNoBreakSpace): Added.
(WebCore::capitalize): Use Vector instead of StringBuffer. Simplify code by
using convertNoBreakSpace function. Removed code that was using StringImpl
directly for a tiny speed boost; if we want to optimize the performance of
this function we would need to do more than that. Return the original string
if it happens to already be capitalized.

Source/WTF:

  • wtf/URL.cpp: Remove a now-inaccurate comment mentioning StringBuffer.
  • wtf/text/StringView.cpp:

(WTF::convertASCIICase): Use createUninitialized instead of StringBuffer.

Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r243115 r243118  
     12019-03-18  Darin Adler  <darin@apple.com>
     2
     3        Cut down on use of StringBuffer, possibly leading toward removing it entirely
     4        https://bugs.webkit.org/show_bug.cgi?id=195870
     5
     6        Reviewed by Daniel Bates.
     7
     8        * wtf/URL.cpp: Remove a now-inaccurate comment mentioning StringBuffer.
     9
     10        * wtf/text/StringView.cpp:
     11        (WTF::convertASCIICase): Use createUninitialized instead of StringBuffer.
     12
    1132019-03-18  Xan Lopez  <xan@igalia.com>
    214

  • trunk/Source/WTF/wtf/URL.cpp

    r241256 r243118  
    11/*
    2  * Copyright (C) 2004, 2007-2008, 2011-2013, 2015-2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
    33 * Copyright (C) 2012 Research In Motion Limited. All rights reserved.
    44 *
    … …  
    4040#include <wtf/text/StringHash.h>
    4141#include <wtf/text/TextStream.h>
    42 
    43 // FIXME: This file makes too much use of the + operator on String.
    44 // We either have to optimize that operator so it doesn't involve
    45 // so many allocations, or change this to use StringBuffer instead.
    46 
    4742
    4843namespace WTF {

  • trunk/Source/WTF/wtf/text/StringView.cpp

    r243049 r243118  
    3535#include <wtf/NeverDestroyed.h>
    3636#include <wtf/Optional.h>
    37 #include <wtf/text/StringBuffer.h>
    3837#include <wtf/text/TextBreakIterator.h>
    3938#include <wtf/unicode/UTF8Conversion.h>
    … …  
    221220        return { };
    222221
    223     StringBuffer<CharacterType> buffer(length);
    224     CharacterType* characters = buffer.characters();
     222    CharacterType* characters;
     223    auto result = String::createUninitialized(length, characters);
    225224    for (unsigned i = 0; i < length; ++i)
    226225        characters[i] = type == ASCIICase::Lower ? toASCIILower(input[i]) : toASCIIUpper(input[i]);
    227     return String::adopt(WTFMove(buffer));
     226    return result;
    228227}
    229228

  • trunk/Source/WebCore/ChangeLog

    r243116 r243118  
     12019-03-18  Darin Adler  <darin@apple.com>
     2
     3        Cut down on use of StringBuffer, possibly leading toward removing it entirely
     4        https://bugs.webkit.org/show_bug.cgi?id=195870
     5
     6        Reviewed by Daniel Bates.
     7
     8        * dom/Document.cpp:
     9        (WebCore::canonicalizedTitle): Fixed all the problems mentioned in "FIXME".
     10        Made this a single function rather than a function template. Switch to
     11        StringBuilder instead of StringBuffer. Return the original string if the
     12        canonicalize operation doesn't change anything.
     13        (WebCore::Document::updateTitle): Updated for the change above.
     14
     15        * platform/Length.cpp:
     16        (WebCore::newCoordsArray): Use createUninitialized instead of StringBuffer.
     17        Also got rid of unneeded use of upconvertedCharacters on a temporary string
     18        that we explicitly created with 16-bit characters. The performance of this
     19        function could be considerably simplified by not copying the original string
     20        at all, but didn't do that at this time.
     21
     22        * platform/text/TextCodecUTF16.cpp:
     23        (WebCore::TextCodecUTF16::decode): Use createUninitialized instead of
     24        StringBuffer. Also renamed numChars to numCodeUnits to both switch to complete
     25        words and to be slightly more accurate.
     26
     27        * rendering/RenderText.cpp:
     28        (WebCore::convertNoBreakSpace): Added.
     29        (WebCore::capitalize): Use Vector instead of StringBuffer. Simplify code by
     30        using convertNoBreakSpace function. Removed code that was using StringImpl
     31        directly for a tiny speed boost; if we want to optimize the performance of
     32        this function we would need to do more than that. Return the original string
     33        if it happens to already be capitalized.
     34
    1352019-03-18  Timothy Hatcher  <timothy@apple.com>
    236

  • trunk/Source/WebCore/dom/Document.cpp

    r243075 r243118  
    44 *           (C) 2001 Dirk Mueller (mueller@kde.org)
    55 *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
    6  * Copyright (C) 2004-2018 Apple Inc. All rights reserved.
     6 * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
    77 * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
    88 * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
    … …  
    15041504}
    15051505
    1506 template<typename CharacterType> static inline String canonicalizedTitle(Document& document, const String& title)
    1507 {
    1508     // FIXME: Compiling a separate copy of this for LChar and UChar is likely unnecessary.
    1509     // FIXME: Missing an optimized case for when title is fine as-is. This unnecessarily allocates
    1510     // and keeps around a new copy, and it's even the less optimal type of StringImpl with a separate buffer.
    1511     // Could probably just use StringBuilder instead.
    1512 
    1513     auto* characters = title.characters<CharacterType>();
    1514     unsigned length = title.length();
    1515 
    1516     StringBuffer<CharacterType> buffer { length };
    1517     unsigned bufferLength = 0;
    1518 
    1519     auto* decoder = document.decoder();
    1520     auto backslashAsCurrencySymbol = decoder ? decoder->encoding().backslashAsCurrencySymbol() : '\\';
    1521 
     1506static String canonicalizedTitle(Document& document, const String& title)
     1507{
    15221508    // Collapse runs of HTML spaces into single space characters.
    15231509    // Strip leading and trailing spaces.
    15241510    // Replace backslashes with currency symbols.
     1511
     1512    StringBuilder builder;
     1513
     1514    auto* decoder = document.decoder();
     1515    auto backslashAsCurrencySymbol = decoder ? decoder->encoding().backslashAsCurrencySymbol() : '\\';
     1516
    15251517    bool previousCharacterWasHTMLSpace = false;
    1526     for (unsigned i = 0; i < length; ++i) {
    1527         auto character = characters[i];
     1518    for (auto character : StringView { title }.codeUnits()) {
    15281519        if (isHTMLSpace(character))
    15291520            previousCharacterWasHTMLSpace = true;
    … …  
    15311522            if (character == '\\')
    15321523                character = backslashAsCurrencySymbol;
    1533             if (previousCharacterWasHTMLSpace && bufferLength)
    1534                 buffer[bufferLength++] = ' ';
    1535             buffer[bufferLength++] = character;
     1524            if (previousCharacterWasHTMLSpace && !builder.isEmpty())
     1525                builder.append(' ');
     1526            builder.append(character);
    15361527            previousCharacterWasHTMLSpace = false;
    15371528        }
    15381529    }
    1539     if (!bufferLength)
    1540         return { };
    1541 
    1542     buffer.shrink(bufferLength);
    1543     return String::adopt(WTFMove(buffer));
     1530
     1531    return builder == title ? title : builder.toString();
    15441532}
    15451533
    … …  
    15501538
    15511539    m_rawTitle = title;
    1552     m_title = title;
    1553 
    1554     if (!m_title.string.isEmpty()) {
    1555         if (m_title.string.is8Bit())
    1556             m_title.string = canonicalizedTitle<LChar>(*this, m_title.string);
    1557         else
    1558             m_title.string = canonicalizedTitle<UChar>(*this, m_title.string);
    1559     }
     1540
     1541    m_title.string = canonicalizedTitle(*this, title.string);
     1542    m_title.direction = title.direction;
    15601543
    15611544    if (auto* loader = this->loader())

  • trunk/Source/WebCore/platform/Length.cpp

    r240641 r243118  
    33 *           (C) 1999 Antti Koivisto (koivisto@kde.org)
    44 *           (C) 2001 Dirk Mueller ( mueller@kde.org )
    5  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2014 Apple Inc. All rights reserved.
     5 * Copyright (C) 2003-2019 Apple Inc. All rights reserved.
    66 * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
    77 *
    … …  
    3232#include <wtf/NeverDestroyed.h>
    3333#include <wtf/StdLibExtras.h>
    34 #include <wtf/text/StringBuffer.h>
    3534#include <wtf/text/StringView.h>
    3635#include <wtf/text/TextStream.h>
    37 
    3836
    3937namespace WebCore {
    … …  
    9290{
    9391    unsigned length = string.length();
    94     StringBuffer<UChar> spacified(length);
     92    UChar* spacified;
     93    auto str = StringImpl::createUninitialized(length, spacified);
    9594    for (unsigned i = 0; i < length; i++) {
    9695        UChar cc = string[i];
    … …  
    10099            spacified[i] = cc;
    101100    }
    102     RefPtr<StringImpl> str = StringImpl::adopt(WTFMove(spacified));
    103101
    104102    str = str->simplifyWhiteSpace();
    105103
    106     len = countCharacter(*str, ' ') + 1;
     104    len = countCharacter(str, ' ') + 1;
    107105    auto r = makeUniqueArray<Length>(len);
    108106
    … …  
    111109    size_t pos2;
    112110
    113     auto upconvertedCharacters = StringView(str.get()).upconvertedCharacters();
    114111    while ((pos2 = str->find(' ', pos)) != notFound) {
    115         r[i++] = parseLength(upconvertedCharacters + pos, pos2 - pos);
     112        r[i++] = parseLength(str->characters16() + pos, pos2 - pos);
    116113        pos = pos2+1;
    117114    }
    118     r[i] = parseLength(upconvertedCharacters + pos, str->length() - pos);
     115    r[i] = parseLength(str->characters16() + pos, str->length() - pos);
    119116
    120117    ASSERT(i == len - 1);

  • trunk/Source/WebCore/platform/text/TextCodecUTF16.cpp

    r225618 r243118  
    11/*
    2  * Copyright (C) 2004-2017 Apple Inc. All rights reserved.
     2 * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2828
    2929#include <wtf/text/CString.h>
    30 #include <wtf/text/StringBuffer.h>
    3130#include <wtf/text/WTFString.h>
    3231
    … …  
    7271    const unsigned char* p = reinterpret_cast<const unsigned char*>(bytes);
    7372    size_t numBytes = length + m_haveBufferedByte;
    74     size_t numChars = numBytes / 2;
     73    size_t numCodeUnits = numBytes / 2;
     74    RELEASE_ASSERT(numCodeUnits <= std::numeric_limits<unsigned>::max());
    7575
    76     StringBuffer<UChar> buffer(numChars);
    77     UChar* q = buffer.characters();
     76    UChar* q;
     77    auto result = String::createUninitialized(numCodeUnits, q);
    7878
    7979    if (m_haveBufferedByte) {
    … …  
    8686        m_haveBufferedByte = false;
    8787        p += 1;
    88         numChars -= 1;
     88        numCodeUnits -= 1;
    8989    }
    9090
    9191    if (m_littleEndian) {
    92         for (size_t i = 0; i < numChars; ++i) {
     92        for (size_t i = 0; i < numCodeUnits; ++i) {
    9393            UChar c = p[0] | (p[1] << 8);
    9494            p += 2;
    … …  
    9696        }
    9797    } else {
    98         for (size_t i = 0; i < numChars; ++i) {
     98        for (size_t i = 0; i < numCodeUnits; ++i) {
    9999            UChar c = (p[0] << 8) | p[1];
    100100            p += 2;
    … …  
    109109    }
    110110
    111     buffer.shrink(q - buffer.characters());
    112 
    113     return String::adopt(WTFMove(buffer));
     111    return result;
    114112}
    115113

  • trunk/Source/WebCore/rendering/RenderText.cpp

    r240641 r243118  
    22 * (C) 1999 Lars Knoll (knoll@kde.org)
    33 * (C) 2000 Dirk Mueller (mueller@kde.org)
    4  * Copyright (C) 2004-2007, 2013-2015 Apple Inc. All rights reserved.
     4 * Copyright (C) 2004-2019 Apple Inc. All rights reserved.
    55 * Copyright (C) 2006 Andrew Wellington (proton@wiretapped.net)
    66 * Copyright (C) 2006 Graham Dennis (graham.dennis@gmail.com)
    … …  
    5252#include <wtf/IsoMallocInlines.h>
    5353#include <wtf/NeverDestroyed.h>
    54 #include <wtf/text/StringBuffer.h>
    5554#include <wtf/text/StringBuilder.h>
    5655#include <wtf/text/TextBreakIterator.h>
    … …  
    142141}
    143142
     143static inline UChar convertNoBreakSpace(UChar character)
     144{
     145    return character == noBreakSpace ? ' ' : character;
     146}
     147
    144148String capitalize(const String& string, UChar previousCharacter)
    145149{
    146     // FIXME: Need to change this to use u_strToTitle instead of u_totitle and to consider locale.
    147 
    148     if (string.isNull())
    149         return string;
     150    // FIXME: Change this to use u_strToTitle instead of u_totitle and to consider locale.
    150151
    151152    unsigned length = string.length();
    152     auto& stringImpl = *string.impl();
    153 
    154     if (length >= std::numeric_limits<unsigned>::max())
    155         CRASH();
    156 
    157     StringBuffer<UChar> stringWithPrevious(length + 1);
    158     stringWithPrevious[0] = previousCharacter == noBreakSpace ? ' ' : previousCharacter;
    159     for (unsigned i = 1; i < length + 1; i++) {
    160         // Replace NO BREAK SPACE with a real space since ICU does not treat it as a word separator.
    161         if (stringImpl[i - 1] == noBreakSpace)
    162             stringWithPrevious[i] = ' ';
    163         else
    164             stringWithPrevious[i] = stringImpl[i - 1];
    165     }
    166 
    167     auto* boundary = wordBreakIterator(StringView(stringWithPrevious.characters(), length + 1));
     153
     154    // Prepend the previous character, and convert NO BREAK SPACE to SPACE so ICU will see a word separator.
     155    Vector<UChar> wordBreakCharacters;
     156    wordBreakCharacters.grow(length + 1);
     157    wordBreakCharacters[0] = convertNoBreakSpace(previousCharacter);
     158    for (unsigned i = 1; i < length + 1; i++)
     159        wordBreakCharacters[i] = convertNoBreakSpace(string[i - 1]);
     160
     161    auto* boundary = wordBreakIterator(StringView { wordBreakCharacters.data(), length + 1 });
    168162    if (!boundary)
    169163        return string;
    … …  
    176170    for (endOfWord = ubrk_next(boundary); endOfWord != UBRK_DONE; startOfWord = endOfWord, endOfWord = ubrk_next(boundary)) {
    177171        if (startOfWord) // Ignore first char of previous string
    178             result.append(stringImpl[startOfWord - 1] == noBreakSpace ? noBreakSpace : u_totitle(stringWithPrevious[startOfWord]));
     172            result.append(string[startOfWord - 1] == noBreakSpace ? noBreakSpace : u_totitle(wordBreakCharacters[startOfWord]));
    179173        for (int i = startOfWord + 1; i < endOfWord; i++)
    180             result.append(stringImpl[i - 1]);
    181     }
    182 
    183     return result.toString();
     174            result.append(string[i - 1]);
     175    }
     176
     177    return result == string ? string : result.toString();
    184178}
    185179
Note: See TracChangeset for help on using the changeset viewer.