Changeset 161840 in webkit for trunk/Source/JavaScriptCore


Ignore:
Timestamp:
Jan 12, 2014, 3:23:44 PM (11 years ago)
Author:
Darin Adler
Message:

Reduce use of String::characters
​https://bugs.webkit.org/show_bug.cgi?id=126854

Reviewed by Sam Weinig.

Source/JavaScriptCore:

  • API/JSValueRef.cpp:

(JSValueMakeFromJSONString): Use characters16 instead of characters for 16-bit case.
Had to remove length check because an empty string could be either 8 bit or 16 bit.
Don't need a null string check before calling is8Bit because JSStringRef can't hold
a null string.

  • bindings/ScriptValue.cpp:

(Deprecated::jsToInspectorValue): Use the existing string here instead of creating
a new one by calling characters and length on the old string. I think this may be
left over from when string types were not the same in JavaScriptCore and WebCore.
Also rewrite the property names loop to use modern for syntax and fewer locals.

  • inspector/InspectorValues.cpp:

(Inspector::escapeChar): Changed to use appendLiteral instead of hard-coding string
lengths. Moved handling of "<" and ">" in here instead of at the call site.
(Inspector::doubleQuoteString): Simplify the code so there is no use of characters
and length. This is still an inefficient way of doing this job and could use a rethink.

  • runtime/DatePrototype.cpp:

(JSC::formatLocaleDate): Use RetainPtr, createCFString, and the conversion from
CFStringRef to WTF::String to remove a lot of unneeded code.

  • runtime/Identifier.h: Removed unneeded Identifier::characters function.
  • runtime/JSStringBuilder.h:

(JSC::JSStringBuilder::append): Use characters16 instead of characters function here,
since we have already checked is8Bit above.

Source/WebCore:

  • bindings/objc/WebScriptObject.mm:

(+[WebScriptObject _convertValueToObjcValue:JSC::originRootObject:rootObject:]):
Get rid of unneeded code to turn a WTF::String into an NSString, since that's
built into the WTF::String class.

  • editing/CompositeEditCommand.cpp:

(WebCore::containsOnlyWhitespace): Use WTF::String's [] operator instead of
an explicit call to the characters function. Small performance cost.

  • editing/TypingCommand.cpp:

(WebCore::TypingCommand::insertText): Ditto.

  • editing/VisibleUnits.cpp:

(WebCore::startOfParagraph): Use reverseFind instead of writing our own loop.
(WebCore::endOfParagraph): Use find instead of writing our own loop.

  • html/parser/HTMLParserIdioms.cpp:

(WebCore::stripLeadingAndTrailingHTMLSpaces): Use characters16 instead of
characters since we have already checked is8Bit.
(WebCore::parseHTMLNonNegativeInteger): Ditto. Replace the length check with
a check of isNull since a zero length string could be 8 bit, but it's not
safe to call is8Bit on a null WTF::String. (That should probably be fixed.)

  • inspector/ContentSearchUtils.cpp:

(WebCore::ContentSearchUtils::createSearchRegexSource): Use StringBuilder
instead of String in code that builds up a string one character at a time.
The old way was ridiculously slow because it allocated a new string for every
character; the new way still isn't all that efficient, but it's better. Also
changed to use strchr instead of WTF::String::find for special characters.

  • inspector/InspectorStyleSheet.cpp:

(WebCore::InspectorStyle::newLineAndWhitespaceDelimiters): Use WTF::String's
[] operator instead of an explicit call to the characters function.

  • inspector/InspectorStyleTextEditor.cpp:

(WebCore::InspectorStyleTextEditor::insertProperty): Ditto.
(WebCore::InspectorStyleTextEditor::internalReplaceProperty): Ditto.

  • platform/Length.cpp:

(WebCore::newCoordsArray): Ditto.

  • platform/LinkHash.cpp:

(WebCore::visitedLinkHash): Use characters16 instead of characters since
we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

  • platform/graphics/Color.cpp:

(WebCore::Color::parseHexColor): Use characters16 since we already checked is8Bit.
(WebCore::Color::Color): Ditto.

  • platform/graphics/TextRun.h:

(WebCore::TextRun::TextRun): Use characters16 instead of characters since
we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

  • platform/text/TextEncodingRegistry.cpp:

(WebCore::atomicCanonicalTextEncodingName): Use characters16 instead of
characters since we already checked is8Bit. Also use isEmpty instead of !length.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::constructTextRun): Use characters16 instead of characters
since we have already checked is8Bit. Replace the length check with a check of
isNull (as above in parseHTMLNonNegativeInteger).

  • rendering/RenderCombineText.cpp:

(WebCore::RenderCombineText::width): Check for zero length instead of checking
for null characters.

  • svg/SVGFontElement.cpp:

(WebCore::SVGFontElement::registerLigaturesInGlyphCache): Rewrite ligatures
for loop and give local variables a better name. Construct the substring with
the substring function. Still a really inefficient approach -- allocating a new
string for every character is absurdly expensive.

  • xml/XPathFunctions.cpp:

(WebCore::XPath::FunId::evaluate): Use String instead of StringBuilder. Still
not all that efficient, but better than before. Use substring to construct the
substring.

  • xml/XPathNodeSet.h: Added begin and end functions so we can use a C++11 for

loop with this class.

Source/WTF:

  • wtf/text/StringImpl.cpp:

(WTF::StringImpl::replace): Use characters16 here since is8Bit was already checked.

  • wtf/text/WTFString.h:

(WTF::String::isAllSpecialCharacters): Use characters16 here since is8Bit was
already checked. Also renamed "len" to "length".

Location:
trunk/Source/JavaScriptCore
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/API/JSValueRef.cpp

    r157688 r161840  
    324324    String str = string->string();
    325325    unsigned length = str.length();
    326     if (length && str.is8Bit()) {
     326    if (str.is8Bit()) {
    327327        LiteralParser<LChar> parser(exec, str.characters8(), length, StrictJSON);
    328328        return toRef(exec, parser.tryLiteralParse());
    329329    }
    330     LiteralParser<UChar> parser(exec, str.characters(), length, StrictJSON);
     330    LiteralParser<UChar> parser(exec, str.characters16(), length, StrictJSON);
    331331    return toRef(exec, parser.tryLiteralParse());
    332332}

  • trunk/Source/JavaScriptCore/ChangeLog

    r161820 r161840  
     12014-01-12  Darin Adler  <[email protected]>
     2
     3        Reduce use of String::characters
     4        https://bugs.webkit.org/show_bug.cgi?id=126854
     5
     6        Reviewed by Sam Weinig.
     7
     8        * API/JSValueRef.cpp:
     9        (JSValueMakeFromJSONString): Use characters16 instead of characters for 16-bit case.
     10        Had to remove length check because an empty string could be either 8 bit or 16 bit.
     11        Don't need a null string check before calling is8Bit because JSStringRef can't hold
     12        a null string.
     13
     14        * bindings/ScriptValue.cpp:
     15        (Deprecated::jsToInspectorValue): Use the existing string here instead of creating
     16        a new one by calling characters and length on the old string. I think this may be
     17        left over from when string types were not the same in JavaScriptCore and WebCore.
     18        Also rewrite the property names loop to use modern for syntax and fewer locals.
     19
     20        * inspector/InspectorValues.cpp:
     21        (Inspector::escapeChar): Changed to use appendLiteral instead of hard-coding string
     22        lengths. Moved handling of "<" and ">" in here instead of at the call site.
     23        (Inspector::doubleQuoteString): Simplify the code so there is no use of characters
     24        and length. This is still an inefficient way of doing this job and could use a rethink.
     25
     26        * runtime/DatePrototype.cpp:
     27        (JSC::formatLocaleDate): Use RetainPtr, createCFString, and the conversion from
     28        CFStringRef to WTF::String to remove a lot of unneeded code.
     29
     30        * runtime/Identifier.h: Removed unneeded Identifier::characters function.
     31
     32        * runtime/JSStringBuilder.h:
     33        (JSC::JSStringBuilder::append): Use characters16 instead of characters function here,
     34        since we have already checked is8Bit above.
     35
    1362014-01-12  Andy Estes  <[email protected]>
    237

  • trunk/Source/JavaScriptCore/bindings/ScriptValue.cpp

    r160457 r161840  
    116116    if (value.isNumber())
    117117        return InspectorBasicValue::create(value.asNumber());
    118     if (value.isString()) {
    119         String s = value.getString(scriptState);
    120         return InspectorString::create(String(s.characters(), s.length()));
    121     }
     118    if (value.isString())
     119        return InspectorString::create(value.getString(scriptState));
    122120
    123121    if (value.isObject()) {
    … …  
    139137        PropertyNameArray propertyNames(scriptState);
    140138        object->methodTable()->getOwnPropertyNames(object, scriptState, propertyNames, ExcludeDontEnumProperties);
    141         for (size_t i = 0; i < propertyNames.size(); i++) {
    142             const Identifier& name =  propertyNames[i];
    143             JSValue propertyValue = object->get(scriptState, name);
    144             RefPtr<InspectorValue> inspectorValue = jsToInspectorValue(scriptState, propertyValue, maxDepth);
     139        for (auto& name : propertyNames) {
     140            RefPtr<InspectorValue> inspectorValue = jsToInspectorValue(scriptState, object->get(scriptState, name), maxDepth);
    145141            if (!inspectorValue)
    146142                return nullptr;
    147             inspectorObject->setValue(String(name.characters(), name.length()), inspectorValue);
     143            inspectorObject->setValue(name.string(), inspectorValue);
    148144        }
    149145        return inspectorObject;

  • trunk/Source/JavaScriptCore/inspector/InspectorValues.cpp

    r160457 r161840  
    447447inline bool escapeChar(UChar c, StringBuilder* dst)
    448448{
     449    // Must escape < and > to prevent script execution.
    449450    switch (c) {
    450     case '\b': dst->append("\\b", 2); break;
    451     case '\f': dst->append("\\f", 2); break;
    452     case '\n': dst->append("\\n", 2); break;
    453     case '\r': dst->append("\\r", 2); break;
    454     case '\t': dst->append("\\t", 2); break;
    455     case '\\': dst->append("\\\\", 2); break;
    456     case '"': dst->append("\\\"", 2); break;
     451    case '\b': dst->appendLiteral("\\b"); break;
     452    case '\f': dst->appendLiteral("\\f"); break;
     453    case '\n': dst->appendLiteral("\\n"); break;
     454    case '\r': dst->appendLiteral("\\r"); break;
     455    case '\t': dst->appendLiteral("\\t"); break;
     456    case '\\': dst->appendLiteral("\\\\"); break;
     457    case '"': dst->appendLiteral("\\\""); break;
     458    case '<': dst->appendLiteral("\\u003C"); break;
     459    case '>': dst->appendLiteral("\\u003E"); break;
    457460    default:
    458461        return false;
    … …  
    467470        UChar c = str[i];
    468471        if (!escapeChar(c, dst)) {
    469             if (c < 32 || c > 126 || c == '<' || c == '>') {
    470                 // 1. Escaping <, > to prevent script execution.
    471                 // 2. Technically, we could also pass through c > 126 as UTF8, but this
    472                 //    is also optional.  It would also be a pain to implement here.
    473                 unsigned int symbol = static_cast<unsigned int>(c);
    474                 String symbolCode = String::format("\\u%04X", symbol);
    475                 dst->append(symbolCode.characters(), symbolCode.length());
    476             } else
     472            // We could format c > 126 as UTF-8 instead of escaping them.
     473            if (c >= 32 || c <= 126)
    477474                dst->append(c);
     475            else {
     476                // FIXME: Way too slow to do this by creating and destroying a string each time.
     477                dst->append(String::format("\\u%04X", static_cast<unsigned>(c)));
     478            }
    478479        }
    479480    }

  • trunk/Source/JavaScriptCore/runtime/DatePrototype.cpp

    r156620 r161840  
    163163        timeStyle = styleFromArgString(arg0String, timeStyle);
    164164
    165     CFLocaleRef locale = CFLocaleCopyCurrent();
    166     CFDateFormatterRef formatter = CFDateFormatterCreate(0, locale, dateStyle, timeStyle);
    167     CFRelease(locale);
    168 
    169     if (useCustomFormat) {
    170         CFStringRef customFormatCFString = CFStringCreateWithCharacters(0, customFormatString.characters(), customFormatString.length());
    171         CFDateFormatterSetFormat(formatter, customFormatCFString);
    172         CFRelease(customFormatCFString);
    173     }
    174 
    175     CFStringRef string = CFDateFormatterCreateStringWithAbsoluteTime(0, formatter, floor(timeInMilliseconds / msPerSecond) - kCFAbsoluteTimeIntervalSince1970);
    176 
    177     CFRelease(formatter);
    178 
    179     // We truncate the string returned from CFDateFormatter if it's absurdly long (> 200 characters).
    180     // That's not great error handling, but it just won't happen so it doesn't matter.
    181     UChar buffer[200];
    182     const size_t bufferLength = WTF_ARRAY_LENGTH(buffer);
    183     size_t length = CFStringGetLength(string);
    184     ASSERT(length <= bufferLength);
    185     if (length > bufferLength)
    186         length = bufferLength;
    187     CFStringGetCharacters(string, CFRangeMake(0, length), buffer);
    188 
    189     CFRelease(string);
    190 
    191     return jsNontrivialString(exec, String(buffer, length));
     165    RetainPtr<CFDateFormatterRef> formatter = adoptCF(CFDateFormatterCreate(kCFAllocatorDefault, adoptCF(CFLocaleCopyCurrent()).get(), dateStyle, timeStyle));
     166
     167    if (useCustomFormat)
     168        CFDateFormatterSetFormat(formatter.get(), customFormatString.createCFString().get());
     169
     170    RetainPtr<CFStringRef> string = adoptCF(CFDateFormatterCreateStringWithAbsoluteTime(kCFAllocatorDefault, formatter.get(), floor(timeInMilliseconds / msPerSecond) - kCFAbsoluteTimeIntervalSince1970));
     171
     172    return jsNontrivialString(exec, string.get());
    192173}
    193174

  • trunk/Source/JavaScriptCore/runtime/Identifier.h

    r158498 r161840  
    5656        StringImpl* impl() const { return m_string.impl(); }
    5757       
    58         const UChar* characters() const { return m_string.characters(); }
    5958        int length() const { return m_string.length(); }
    6059       

  • trunk/Source/JavaScriptCore/runtime/JSStringBuilder.h

    r147892 r161840  
    106106            upConvert();
    107107        }
    108         m_okay &= buffer16.tryAppend(str.characters(), length);
     108        m_okay &= buffer16.tryAppend(str.characters16(), length);
    109109    }
    110110
Note: See TracChangeset for help on using the changeset viewer.