Title: [292619] trunk/Source
Revision
292619
Author
cdu...@apple.com
Date
2022-04-08 10:54:47 -0700 (Fri, 08 Apr 2022)

Log Message

Reduce number of StringView to String conversions in JSC
https://bugs.webkit.org/show_bug.cgi?id=238911

Reviewed by Darin Adler.

Source/_javascript_Core:

* dfg/DFGLazyJSValue.cpp:
(JSC::DFG::LazyJSValue::getValue const):
Resolve ambiguity by explicitly converting the StringImpl to a String before
calling jsString(). [1]

* jsc.cpp:
Avoid constructing a String from the StringView, just to compute a hash.
Instead, rely on the StringViewHashTranslator for this.

* profiler/ProfilerOSRExit.cpp:
(JSC::Profiler::OSRExit::toJS const):
exitKindToString() returns an ASCIILiteral whose length is always greater
than 1 so we can call the more efficient jsNontrivialString() instead of
jsString(). Calling jsString() here had become ambiguous because an
ASCIILiteral can be implicitely converted to both a String and a
StringView [2].

* runtime/ArrayPrototype.cpp:
(JSC::fastJoin):
Call the new jsString() overload that takes a StringView, to avoid
unnecessarily constructing a String in the case where the length is <= 1 [3].

* runtime/ErrorInstance.cpp:
(JSC::appendSourceToErrorMessage):
* runtime/ErrorInstance.h:
* runtime/ExceptionHelpers.cpp:
(JSC::defaultApproximateSourceError):
(JSC::defaultSourceAppender):
(JSC::functionCallBase):
(JSC::notAFunctionSourceAppender):
(JSC::invalidParameterInSourceAppender):
(JSC::invalidParameterInstanceofSourceAppender):
(JSC::invalidParameterInstanceofNotFunctionSourceAppender):
(JSC::invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender):
(JSC::invalidPrototypeSourceAppender):
* runtime/ExceptionHelpers.h:
Call SourceAppender with a StringView since this is what we have. In most
cases, these appenders end up calling makeString() and it is thus beneficial
to avoid temporary/intermediate String constructions.

* runtime/FunctionExecutable.cpp:
(JSC::FunctionExecutable::toStringSlow):
Same as [3].

* runtime/IdentifierInlines.h:
(JSC::identifierToJSValue):
(JSC::identifierToSafePublicJSValue):
Same as [1].

* runtime/IntlDateTimeFormat.cpp:
(JSC::IntlDateTimeFormat::formatToParts const):
(JSC::IntlDateTimeFormat::formatRangeToParts):
* runtime/IntlLocale.cpp:
(JSC::IntlLocale::textInfo):
* runtime/IntlNumberFormat.cpp:
(JSC::IntlNumberFormat::formatRangeToPartsInternal):
(JSC::IntlNumberFormat::formatToPartsInternal):
Same as [2].

* runtime/IntlRelativeTimeFormat.cpp:
(JSC::IntlRelativeTimeFormat::formatToParts const):
Same as [3].

* runtime/JSArrayBufferPrototype.cpp:
(JSC::JSArrayBufferPrototype::finishCreation):
Same as [2].

* runtime/JSModuleLoader.cpp:
(JSC::JSModuleLoader::requestImportModule):
(JSC::JSC_DEFINE_HOST_FUNCTION):
Same as [1].

* runtime/JSString.h:
(JSC::jsString):
Add a jsString() overload that takes in a StringView instead of a String.
This avoids construction of a String for call sites having a StringView
in the event where the view's length is <= 1.

* runtime/SymbolConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
Same as [1].

Source/WTF:

Provide a reverseFind(StringView, unsigned) overload on StringView, for consistency
with String and to facilitate the converting of code from String to StringView.

* wtf/text/StringCommon.h:
(WTF::reverseFindInner):
* wtf/text/StringImpl.cpp:
(WTF::reverseFindInner): Deleted.
* wtf/text/StringView.cpp:
(WTF::StringView::reverseFind const):
* wtf/text/StringView.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (292618 => 292619)


--- trunk/Source/_javascript_Core/ChangeLog	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/ChangeLog	2022-04-08 17:54:47 UTC (rev 292619)
@@ -1,3 +1,93 @@
+2022-04-08  Chris Dumez  <cdu...@apple.com>
+
+        Reduce number of StringView to String conversions in JSC
+        https://bugs.webkit.org/show_bug.cgi?id=238911
+
+        Reviewed by Darin Adler.
+
+        * dfg/DFGLazyJSValue.cpp:
+        (JSC::DFG::LazyJSValue::getValue const):
+        Resolve ambiguity by explicitly converting the StringImpl to a String before
+        calling jsString(). [1]
+
+        * jsc.cpp:
+        Avoid constructing a String from the StringView, just to compute a hash.
+        Instead, rely on the StringViewHashTranslator for this.
+
+        * profiler/ProfilerOSRExit.cpp:
+        (JSC::Profiler::OSRExit::toJS const):
+        exitKindToString() returns an ASCIILiteral whose length is always greater
+        than 1 so we can call the more efficient jsNontrivialString() instead of
+        jsString(). Calling jsString() here had become ambiguous because an
+        ASCIILiteral can be implicitely converted to both a String and a
+        StringView [2].
+
+        * runtime/ArrayPrototype.cpp:
+        (JSC::fastJoin):
+        Call the new jsString() overload that takes a StringView, to avoid
+        unnecessarily constructing a String in the case where the length is <= 1 [3].
+
+        * runtime/ErrorInstance.cpp:
+        (JSC::appendSourceToErrorMessage):
+        * runtime/ErrorInstance.h:
+        * runtime/ExceptionHelpers.cpp:
+        (JSC::defaultApproximateSourceError):
+        (JSC::defaultSourceAppender):
+        (JSC::functionCallBase):
+        (JSC::notAFunctionSourceAppender):
+        (JSC::invalidParameterInSourceAppender):
+        (JSC::invalidParameterInstanceofSourceAppender):
+        (JSC::invalidParameterInstanceofNotFunctionSourceAppender):
+        (JSC::invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender):
+        (JSC::invalidPrototypeSourceAppender):
+        * runtime/ExceptionHelpers.h:
+        Call SourceAppender with a StringView since this is what we have. In most
+        cases, these appenders end up calling makeString() and it is thus beneficial
+        to avoid temporary/intermediate String constructions. 
+
+        * runtime/FunctionExecutable.cpp:
+        (JSC::FunctionExecutable::toStringSlow):
+        Same as [3].
+
+        * runtime/IdentifierInlines.h:
+        (JSC::identifierToJSValue):
+        (JSC::identifierToSafePublicJSValue):
+        Same as [1].
+
+        * runtime/IntlDateTimeFormat.cpp:
+        (JSC::IntlDateTimeFormat::formatToParts const):
+        (JSC::IntlDateTimeFormat::formatRangeToParts):
+        * runtime/IntlLocale.cpp:
+        (JSC::IntlLocale::textInfo):
+        * runtime/IntlNumberFormat.cpp:
+        (JSC::IntlNumberFormat::formatRangeToPartsInternal):
+        (JSC::IntlNumberFormat::formatToPartsInternal):
+        Same as [2].
+
+        * runtime/IntlRelativeTimeFormat.cpp:
+        (JSC::IntlRelativeTimeFormat::formatToParts const):
+        Same as [3].
+
+        * runtime/JSArrayBufferPrototype.cpp:
+        (JSC::JSArrayBufferPrototype::finishCreation):
+        Same as [2].
+
+        * runtime/JSModuleLoader.cpp:
+        (JSC::JSModuleLoader::requestImportModule):
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        Same as [1].
+
+        * runtime/JSString.h:
+        (JSC::jsString):
+        Add a jsString() overload that takes in a StringView instead of a String.
+        This avoids construction of a String for call sites having a StringView
+        in the event where the view's length is <= 1.
+
+        * runtime/SymbolConstructor.cpp:
+        (JSC::JSC_DEFINE_HOST_FUNCTION):
+        Same as [1].
+
+
 2022-04-08  Keith Miller  <keith_mil...@apple.com>
 
         Broaden TypedArray API fix to all apps not just Bleacher Report

Modified: trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/dfg/DFGLazyJSValue.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -51,7 +51,7 @@
     case SingleCharacterString:
         return jsSingleCharacterString(vm, u.character);
     case KnownStringImpl:
-        return jsString(vm, u.stringImpl);
+        return jsString(vm, String { u.stringImpl });
     case NewStringImpl:
         return jsString(vm, AtomStringImpl::add(u.stringImpl));
     }

Modified: trunk/Source/_javascript_Core/jsc.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/jsc.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/jsc.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -1096,7 +1096,7 @@
             return { };
         const char* cachePath = Options::diskCachePath();
         String filename = FileSystem::encodeForFileName(FileSystem::lastComponentOfPathIgnoringTrailingSlash(sourceOrigin().url().fileSystemPath()));
-        return FileSystem::pathByAppendingComponent(cachePath, makeString(source().toString().hash(), '-', filename, ".bytecode-cache"));
+        return FileSystem::pathByAppendingComponent(cachePath, makeString(source().hash(), '-', filename, ".bytecode-cache"));
     }
 
     void loadBytecode() const

Modified: trunk/Source/_javascript_Core/profiler/ProfilerOSRExit.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/profiler/ProfilerOSRExit.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/profiler/ProfilerOSRExit.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -51,7 +51,7 @@
     JSObject* result = constructEmptyObject(globalObject);
     result->putDirect(vm, vm.propertyNames->id, jsNumber(m_id));
     result->putDirect(vm, vm.propertyNames->origin, m_origin.toJS(globalObject));
-    result->putDirect(vm, vm.propertyNames->exitKind, jsString(vm, exitKindToString(m_exitKind)));
+    result->putDirect(vm, vm.propertyNames->exitKind, jsNontrivialString(vm, exitKindToString(m_exitKind)));
     result->putDirect(vm, vm.propertyNames->isWatchpoint, jsBoolean(m_isWatchpoint));
     result->putDirect(vm, vm.propertyNames->count, jsNumber(m_counter));
     return result;

Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -544,7 +544,7 @@
             if (length <= 1)
                 return result;
 
-            JSString* operand = jsString(vm, separator.toString());
+            JSString* operand = jsString(vm, separator);
             RETURN_IF_EXCEPTION(scope, { });
             unsigned count = length - 1;
             for (;;) {

Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -97,7 +97,7 @@
         return message;
     
     if (expressionStart < expressionStop)
-        return appender(message, codeBlock->source().provider()->getRange(expressionStart, expressionStop).toString(), type, ErrorInstance::FoundExactSource);
+        return appender(message, codeBlock->source().provider()->getRange(expressionStart, expressionStop), type, ErrorInstance::FoundExactSource);
 
     // No range information, so give a few characters of context.
     int dataLength = sourceString.length();
@@ -113,7 +113,7 @@
         stop++;
     while (stop > expressionStart && isStrWhiteSpace(sourceString[stop - 1]))
         stop--;
-    return appender(message, codeBlock->source().provider()->getRange(start, stop).toString(), type, ErrorInstance::FoundApproximateSource);
+    return appender(message, codeBlock->source().provider()->getRange(start, stop), type, ErrorInstance::FoundApproximateSource);
 }
 
 void ErrorInstance::finishCreation(VM& vm, JSGlobalObject* globalObject, const String& message, JSValue cause, SourceAppender appender, RuntimeType type, bool useCurrentFrame)

Modified: trunk/Source/_javascript_Core/runtime/ErrorInstance.h (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/ErrorInstance.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/ErrorInstance.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -45,7 +45,7 @@
     }
 
     enum SourceTextWhereErrorOccurred { FoundExactSource, FoundApproximateSource };
-    typedef String (*SourceAppender) (const String& originalMessage, const String& sourceText, RuntimeType, SourceTextWhereErrorOccurred);
+    typedef String (*SourceAppender) (const String& originalMessage, StringView sourceText, RuntimeType, SourceTextWhereErrorOccurred);
 
     DECLARE_EXPORT_INFO;
 

Modified: trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -81,12 +81,12 @@
     return StringView(originalMessage).substring(0, maxLength);
 }
 
-static String defaultApproximateSourceError(const String& originalMessage, const String& sourceText)
+static String defaultApproximateSourceError(const String& originalMessage, StringView sourceText)
 {
     return makeString(clampErrorMessage(originalMessage), " (near '...", sourceText, "...')");
 }
 
-String defaultSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+String defaultSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     if (occurrence == ErrorInstance::FoundApproximateSource)
         return defaultApproximateSourceError(originalMessage, sourceText);
@@ -95,7 +95,7 @@
     return makeString(clampErrorMessage(originalMessage), " (evaluating '", sourceText, "')");
 }
 
-static String functionCallBase(const String& sourceText)
+static StringView functionCallBase(StringView sourceText)
 { 
     // This function retrieves the 'foo.bar' substring from 'foo.bar(baz)'.
     // FIXME: This function has simple processing of /* */ style comments.
@@ -153,7 +153,7 @@
     return sourceText.left(idx + 1);
 }
 
-static String notAFunctionSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType type, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+static String notAFunctionSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType type, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     ASSERT(type != TypeFunction);
 
@@ -169,7 +169,7 @@
     else
         displayValue = StringView(originalMessage.characters16(), notAFunctionIndex - 1);
 
-    String base = functionCallBase(sourceText);
+    StringView base = functionCallBase(sourceText);
     if (!base)
         return defaultApproximateSourceError(originalMessage, sourceText);
     StringBuilder builder(StringBuilder::OverflowHandler::RecordOverflow);
@@ -189,7 +189,7 @@
     return builder.toString();
 }
 
-static String invalidParameterInSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType type, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+static String invalidParameterInSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType type, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     ASSERT_UNUSED(type, type != TypeObject);
 
@@ -208,11 +208,11 @@
         return makeString(originalMessage, " (evaluating '", sourceText, "')");
 
     static constexpr unsigned inLength = 2;
-    String rightHandSide = sourceText.substring(inIndex + inLength).simplifyWhiteSpace();
+    String rightHandSide = sourceText.substring(inIndex + inLength).toStringWithoutCopying().simplifyWhiteSpace();
     return makeString(rightHandSide, " is not an Object. (evaluating '", sourceText, "')");
 }
 
-inline String invalidParameterInstanceofSourceAppender(const String& content, const String& originalMessage, const String& sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+inline String invalidParameterInstanceofSourceAppender(const String& content, const String& originalMessage, StringView sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     if (occurrence == ErrorInstance::FoundApproximateSource)
         return defaultApproximateSourceError(originalMessage, sourceText);
@@ -227,21 +227,21 @@
         return makeString(originalMessage, " (evaluating '", sourceText, "')");
 
     static constexpr unsigned instanceofLength = 10;
-    String rightHandSide = sourceText.substring(instanceofIndex + instanceofLength).simplifyWhiteSpace();
+    String rightHandSide = sourceText.substring(instanceofIndex + instanceofLength).toStringWithoutCopying().simplifyWhiteSpace();
     return makeString(rightHandSide, content, ". (evaluating '", sourceText, "')");
 }
 
-static String invalidParameterInstanceofNotFunctionSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+static String invalidParameterInstanceofNotFunctionSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     return invalidParameterInstanceofSourceAppender(" is not a function"_s, originalMessage, sourceText, runtimeType, occurrence);
 }
 
-static String invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+static String invalidParameterInstanceofhasInstanceValueNotFunctionSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType runtimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     return invalidParameterInstanceofSourceAppender("[Symbol.hasInstance] is not a function, undefined, or null"_s, originalMessage, sourceText, runtimeType, occurrence);
 }
 
-static String invalidPrototypeSourceAppender(const String& originalMessage, const String& sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
+static String invalidPrototypeSourceAppender(const String& originalMessage, StringView sourceText, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred occurrence)
 {
     if (occurrence == ErrorInstance::FoundApproximateSource)
         return defaultApproximateSourceError(originalMessage, sourceText);

Modified: trunk/Source/_javascript_Core/runtime/ExceptionHelpers.h (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/ExceptionHelpers.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/ExceptionHelpers.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -38,7 +38,7 @@
 
 typedef JSObject* (*ErrorFactory)(JSGlobalObject*, const String&, ErrorInstance::SourceAppender);
 
-String defaultSourceAppender(const String&, const String&, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred);
+String defaultSourceAppender(const String&, StringView, RuntimeType, ErrorInstance::SourceTextWhereErrorOccurred);
 
 JS_EXPORT_PRIVATE JSObject* createError(JSGlobalObject*, JSValue, const String&, ErrorInstance::SourceAppender);
 JS_EXPORT_PRIVATE JSObject* createStackOverflowError(JSGlobalObject*);

Modified: trunk/Source/_javascript_Core/runtime/FunctionExecutable.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/FunctionExecutable.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/FunctionExecutable.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -181,7 +181,7 @@
         return cacheIfNoException(jsMakeNontrivialString(globalObject, "function ", name().string(), "() {\n    [native code]\n}"));
 
     if (isClass())
-        return cache(jsString(vm, classSource().view().toString()));
+        return cache(jsString(vm, classSource().view()));
 
     ASCIILiteral functionHeader = ""_s;
     switch (parseMode()) {

Modified: trunk/Source/_javascript_Core/runtime/IdentifierInlines.h (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/IdentifierInlines.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/IdentifierInlines.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -129,7 +129,7 @@
 {
     if (identifier.isSymbol())
         return Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()));
-    return jsString(vm, identifier.impl());
+    return jsString(vm, String { identifier.impl() });
 }
 
 inline JSValue identifierToSafePublicJSValue(VM& vm, const Identifier& identifier) 
@@ -136,7 +136,7 @@
 {
     if (identifier.isSymbol() && !identifier.isPrivateName())
         return Symbol::create(vm, static_cast<SymbolImpl&>(*identifier.impl()));
-    return jsString(vm, identifier.impl());
+    return jsString(vm, String { identifier.impl() });
 }
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/IntlDateTimeFormat.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/IntlDateTimeFormat.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/IntlDateTimeFormat.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -1406,7 +1406,7 @@
         previousEndIndex = endIndex;
 
         if (fieldType >= 0) {
-            auto type = jsString(vm, partTypeString(UDateFormatField(fieldType)));
+            auto type = jsNontrivialString(vm, partTypeString(UDateFormatField(fieldType)));
             auto value = jsString(vm, resultString.substring(beginIndex, endIndex - beginIndex));
             JSObject* part = constructEmptyObject(globalObject);
             part->putDirect(vm, vm.propertyNames->type, type);
@@ -1811,7 +1811,7 @@
 
         ASSERT(category == UFIELD_CATEGORY_DATE);
 
-        auto type = jsString(vm, partTypeString(UDateFormatField(fieldType)));
+        auto type = jsNontrivialString(vm, partTypeString(UDateFormatField(fieldType)));
         JSObject* part = createPart(type, beginIndex, endIndex - beginIndex);
         parts->push(globalObject, part);
         RETURN_IF_EXCEPTION(scope, { });

Modified: trunk/Source/_javascript_Core/runtime/IntlLocale.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/IntlLocale.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/IntlLocale.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -743,16 +743,16 @@
     switch (layout) {
     default:
     case ULOC_LAYOUT_LTR:
-        layoutString = jsString(vm, "ltr"_s);
+        layoutString = jsNontrivialString(vm, "ltr"_s);
         break;
     case ULOC_LAYOUT_RTL:
-        layoutString = jsString(vm, "rtl"_s);
+        layoutString = jsNontrivialString(vm, "rtl"_s);
         break;
     case ULOC_LAYOUT_TTB:
-        layoutString = jsString(vm, "ttb"_s);
+        layoutString = jsNontrivialString(vm, "ttb"_s);
         break;
     case ULOC_LAYOUT_BTT:
-        layoutString = jsString(vm, "btt"_s);
+        layoutString = jsNontrivialString(vm, "btt"_s);
         break;
     }
 

Modified: trunk/Source/_javascript_Core/runtime/IntlNumberFormat.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/IntlNumberFormat.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/IntlNumberFormat.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -1137,7 +1137,7 @@
             sign = end.sign();
         }
         auto fieldType = field.m_field;
-        auto partType = fieldType == literalField ? literalString : jsString(vm, partTypeString(UNumberFormatFields(fieldType), style, sign, numberType));
+        auto partType = fieldType == literalField ? literalString : jsNontrivialString(vm, partTypeString(UNumberFormatFields(fieldType), style, sign, numberType));
         JSObject* part = createPart(partType, field.m_range.begin(), field.m_range.distance());
         parts->push(globalObject, part);
         RETURN_IF_EXCEPTION(scope, void());
@@ -1507,7 +1507,7 @@
 
     for (auto& field : flatten) {
         auto fieldType = field.m_field;
-        auto partType = fieldType == literalField ? literalString : jsString(vm, partTypeString(UNumberFormatFields(fieldType), style, sign, numberType));
+        auto partType = fieldType == literalField ? literalString : jsNontrivialString(vm, partTypeString(UNumberFormatFields(fieldType), style, sign, numberType));
         auto partValue = jsSubstring(vm, formatted, field.m_range.begin(), field.m_range.distance());
         JSObject* part = constructEmptyObject(globalObject);
         part->putDirect(vm, vm.propertyNames->type, partType);

Modified: trunk/Source/_javascript_Core/runtime/IntlRelativeTimeFormat.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/IntlRelativeTimeFormat.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/IntlRelativeTimeFormat.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -318,7 +318,7 @@
         }
 
         IntlFieldIterator fieldIterator(*iterator.get());
-        IntlNumberFormat::formatToPartsInternal(globalObject, IntlNumberFormat::Style::Decimal, std::signbit(absValue), IntlMathematicalValue::numberTypeFromDouble(absValue), formattedNumber, fieldIterator, parts, nullptr, jsString(vm, singularUnit(unit).toString()));
+        IntlNumberFormat::formatToPartsInternal(globalObject, IntlNumberFormat::Style::Decimal, std::signbit(absValue), IntlMathematicalValue::numberTypeFromDouble(absValue), formattedNumber, fieldIterator, parts, nullptr, jsString(vm, singularUnit(unit)));
         RETURN_IF_EXCEPTION(scope, { });
     }
 

Modified: trunk/Source/_javascript_Core/runtime/JSArrayBufferPrototype.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/JSArrayBufferPrototype.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/JSArrayBufferPrototype.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -269,7 +269,7 @@
 {
     Base::finishCreation(vm);
     
-    putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsString(vm, arrayBufferSharingModeName(sharingMode)), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
+    putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, jsNontrivialString(vm, arrayBufferSharingModeName(sharingMode)), PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
     if (sharingMode == ArrayBufferSharingMode::Default) {
         JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->slice, arrayBufferProtoFuncSlice, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);
         JSC_NATIVE_GETTER_WITHOUT_TRANSITION(vm.propertyNames->byteLength, arrayBufferProtoGetterFuncByteLength, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);

Modified: trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/JSModuleLoader.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -232,7 +232,7 @@
     ASSERT(callData.type != CallData::Type::None);
 
     MarkedArgumentBuffer arguments;
-    arguments.append(jsString(vm, moduleKey.impl()));
+    arguments.append(jsString(vm, String { moduleKey.impl() }));
     arguments.append(parameters);
     arguments.append(scriptFetcher);
     ASSERT(!arguments.hasOverflowed());
@@ -398,7 +398,7 @@
     RETURN_IF_EXCEPTION(scope, encodedJSValue());
     size_t i = 0;
     for (auto& key : moduleRecord->requestedModules()) {
-        result->putDirectIndex(globalObject, i++, jsString(vm, key.get()));
+        result->putDirectIndex(globalObject, i++, jsString(vm, String { key.get() }));
         RETURN_IF_EXCEPTION(scope, encodedJSValue());
     }
     return JSValue::encode(result);

Modified: trunk/Source/_javascript_Core/runtime/JSString.h (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/JSString.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/JSString.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -49,6 +49,8 @@
 
 JSString* jsEmptyString(VM&);
 JSString* jsString(VM&, const String&); // returns empty string if passed null string
+JSString* jsString(VM&, String&&); // returns empty string if passed null string
+JSString* jsString(VM&, StringView); // returns empty string if passed null string
 
 JSString* jsSingleCharacterString(VM&, UChar);
 JSString* jsSubstring(VM&, const String&, unsigned offset, unsigned length);
@@ -251,6 +253,8 @@
     void swapToAtomString(VM&, RefPtr<AtomStringImpl>&&) const;
 
     friend JSString* jsString(VM&, const String&);
+    friend JSString* jsString(VM&, String&&);
+    friend JSString* jsString(VM&, StringView);
     friend JSString* jsString(JSGlobalObject*, JSString*, JSString*);
     friend JSString* jsString(JSGlobalObject*, const String&, JSString*);
     friend JSString* jsString(JSGlobalObject*, JSString*, const String&);
@@ -876,6 +880,33 @@
     return JSString::create(vm, *s.impl());
 }
 
+inline JSString* jsString(VM& vm, String&& s)
+{
+    int size = s.length();
+    if (!size)
+        return vm.smallStrings.emptyString();
+    if (size == 1) {
+        UChar c = s.characterAt(0);
+        if (c <= maxSingleCharacterString)
+            return vm.smallStrings.singleCharacterString(c);
+    }
+    return JSString::create(vm, s.releaseImpl().releaseNonNull());
+}
+
+inline JSString* jsString(VM& vm, StringView s)
+{
+    int size = s.length();
+    if (!size)
+        return vm.smallStrings.emptyString();
+    if (size == 1) {
+        UChar c = s.characterAt(0);
+        if (c <= maxSingleCharacterString)
+            return vm.smallStrings.singleCharacterString(c);
+    }
+    auto impl = s.is8Bit() ? StringImpl::create(s.characters8(), s.length()) : StringImpl::create(s.characters16(), s.length());
+    return JSString::create(vm, WTFMove(impl));
+}
+
 inline JSString* jsSubstring(VM& vm, JSGlobalObject* globalObject, JSString* base, unsigned offset, unsigned length)
 {
     auto scope = DECLARE_THROW_SCOPE(vm);

Modified: trunk/Source/_javascript_Core/runtime/SymbolConstructor.cpp (292618 => 292619)


--- trunk/Source/_javascript_Core/runtime/SymbolConstructor.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/_javascript_Core/runtime/SymbolConstructor.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -125,7 +125,7 @@
         return JSValue::encode(jsUndefined());
 
     ASSERT(uid.symbolRegistry() == &vm.symbolRegistry());
-    return JSValue::encode(jsString(vm, &uid));
+    return JSValue::encode(jsString(vm, String { uid }));
 }
 
 } // namespace JSC

Modified: trunk/Source/WTF/ChangeLog (292618 => 292619)


--- trunk/Source/WTF/ChangeLog	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/ChangeLog	2022-04-08 17:54:47 UTC (rev 292619)
@@ -1,3 +1,21 @@
+2022-04-08  Chris Dumez  <cdu...@apple.com>
+
+        Reduce number of StringView to String conversions in JSC
+        https://bugs.webkit.org/show_bug.cgi?id=238911
+
+        Reviewed by Darin Adler.
+
+        Provide a reverseFind(StringView, unsigned) overload on StringView, for consistency
+        with String and to facilitate the converting of code from String to StringView.
+
+        * wtf/text/StringCommon.h:
+        (WTF::reverseFindInner):
+        * wtf/text/StringImpl.cpp:
+        (WTF::reverseFindInner): Deleted.
+        * wtf/text/StringView.cpp:
+        (WTF::StringView::reverseFind const):
+        * wtf/text/StringView.h:
+
 2022-04-08  Keith Miller  <keith_mil...@apple.com>
 
         Broaden TypedArray API fix to all apps not just Bleacher Report

Modified: trunk/Source/WTF/wtf/text/StringCommon.h (292618 => 292619)


--- trunk/Source/WTF/wtf/text/StringCommon.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/wtf/text/StringCommon.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -458,6 +458,33 @@
     return find(characters, length, static_cast<LChar>(matchCharacter), index);
 }
 
+template <typename SearchCharacterType, typename MatchCharacterType>
+ALWAYS_INLINE static size_t reverseFindInner(const SearchCharacterType* searchCharacters, const MatchCharacterType* matchCharacters, unsigned start, unsigned length, unsigned matchLength)
+{
+    // Optimization: keep a running hash of the strings,
+    // only call equal if the hashes match.
+
+    // delta is the number of additional times to test; delta == 0 means test only once.
+    unsigned delta = std::min(start, length - matchLength);
+
+    unsigned searchHash = 0;
+    unsigned matchHash = 0;
+    for (unsigned i = 0; i < matchLength; ++i) {
+        searchHash += searchCharacters[delta + i];
+        matchHash += matchCharacters[i];
+    }
+
+    // keep looping until we match
+    while (searchHash != matchHash || !equal(searchCharacters + delta, matchCharacters, matchLength)) {
+        if (!delta)
+            return notFound;
+        --delta;
+        searchHash -= searchCharacters[delta + matchLength];
+        searchHash += searchCharacters[delta];
+    }
+    return delta;
+}
+
 // This is marked inline since it's mostly used in non-inline functions for each string type.
 // When used directly in code it's probably OK to be inline; maybe the loop will be unrolled.
 template<typename CharacterType> inline bool equalLettersIgnoringASCIICase(const CharacterType* characters, const char* lowercaseLetters, unsigned length)

Modified: trunk/Source/WTF/wtf/text/StringHash.h (292618 => 292619)


--- trunk/Source/WTF/wtf/text/StringHash.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/wtf/text/StringHash.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -205,9 +205,7 @@
     struct StringViewHashTranslator {
         static unsigned hash(StringView key)
         {
-            if (key.is8Bit())
-                return StringHasher::computeHashAndMaskTop8Bits(key.characters8(), key.length());
-            return StringHasher::computeHashAndMaskTop8Bits(key.characters16(), key.length());
+            return key.hash();
         }
 
         static bool equal(const String& a, StringView b)

Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (292618 => 292619)


--- trunk/Source/WTF/wtf/text/StringImpl.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -983,33 +983,6 @@
     return WTF::reverseFind(characters16(), m_length, character, start);
 }
 
-template <typename SearchCharacterType, typename MatchCharacterType>
-ALWAYS_INLINE static size_t reverseFindInner(const SearchCharacterType* searchCharacters, const MatchCharacterType* matchCharacters, unsigned start, unsigned length, unsigned matchLength)
-{
-    // Optimization: keep a running hash of the strings,
-    // only call equal if the hashes match.
-
-    // delta is the number of additional times to test; delta == 0 means test only once.
-    unsigned delta = std::min(start, length - matchLength);
-    
-    unsigned searchHash = 0;
-    unsigned matchHash = 0;
-    for (unsigned i = 0; i < matchLength; ++i) {
-        searchHash += searchCharacters[delta + i];
-        matchHash += matchCharacters[i];
-    }
-
-    // keep looping until we match
-    while (searchHash != matchHash || !equal(searchCharacters + delta, matchCharacters, matchLength)) {
-        if (!delta)
-            return notFound;
-        --delta;
-        searchHash -= searchCharacters[delta + matchLength];
-        searchHash += searchCharacters[delta];
-    }
-    return delta;
-}
-
 size_t StringImpl::reverseFind(StringView matchString, unsigned start)
 {
     // Check for null or empty string to match against

Modified: trunk/Source/WTF/wtf/text/StringView.cpp (292618 => 292619)


--- trunk/Source/WTF/wtf/text/StringView.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/wtf/text/StringView.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -359,6 +359,31 @@
     return stripLeadingAndTrailingMatchedCharacters(isASCIISpace<UChar>);
 }
 
+size_t StringView::reverseFind(StringView matchString, unsigned start) const
+{
+    if (isNull())
+        return notFound;
+
+    unsigned matchLength = matchString.length();
+    unsigned ourLength = length();
+    if (!matchLength)
+        return std::min(start, ourLength);
+
+    // Check start & matchLength are in range.
+    if (matchLength > ourLength)
+        return notFound;
+
+    if (is8Bit()) {
+        if (matchString.is8Bit())
+            return reverseFindInner(characters8(), matchString.characters8(), start, ourLength, matchLength);
+        return reverseFindInner(characters8(), matchString.characters16(), start, ourLength, matchLength);
+    }
+
+    if (matchString.is8Bit())
+        return reverseFindInner(characters16(), matchString.characters8(), start, ourLength, matchLength);
+    return reverseFindInner(characters16(), matchString.characters16(), start, ourLength, matchLength);
+}
+
 int codePointCompare(StringView lhs, StringView rhs)
 {
     bool lhsIs8Bit = lhs.is8Bit();

Modified: trunk/Source/WTF/wtf/text/StringView.h (292618 => 292619)


--- trunk/Source/WTF/wtf/text/StringView.h	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WTF/wtf/text/StringView.h	2022-04-08 17:54:47 UTC (rev 292619)
@@ -94,6 +94,8 @@
     const LChar* characters8() const;
     const UChar* characters16() const;
 
+    unsigned hash() const;
+
     // Return characters8() or characters16() depending on CharacterType.
     template<typename CharacterType> const CharacterType* characters() const;
 
@@ -147,6 +149,7 @@
     WTF_EXPORT_PRIVATE size_t find(StringView, unsigned start = 0) const;
 
     size_t reverseFind(UChar, unsigned index = std::numeric_limits<unsigned>::max()) const;
+    WTF_EXPORT_PRIVATE size_t reverseFind(StringView, unsigned start = std::numeric_limits<unsigned>::max()) const;
 
     WTF_EXPORT_PRIVATE size_t findIgnoringASCIICase(StringView) const;
     WTF_EXPORT_PRIVATE size_t findIgnoringASCIICase(StringView, unsigned start) const;
@@ -436,6 +439,13 @@
     return static_cast<const UChar*>(m_characters);
 }
 
+inline unsigned StringView::hash() const
+{
+    if (is8Bit())
+        return StringHasher::computeHashAndMaskTop8Bits(characters8(), length());
+    return StringHasher::computeHashAndMaskTop8Bits(characters16(), length());
+}
+
 template<> ALWAYS_INLINE const LChar* StringView::characters<LChar>() const
 {
     return characters8();

Modified: trunk/Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp (292618 => 292619)


--- trunk/Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp	2022-04-08 17:53:28 UTC (rev 292618)
+++ trunk/Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp	2022-04-08 17:54:47 UTC (rev 292619)
@@ -2065,13 +2065,13 @@
     JSC::JSObject* processTargetsObject = JSC::constructEmptyArray(globalObject, nullptr);
     RETURN_IF_EXCEPTION(scope, JSValueMakeUndefined(context));
     int index = 0;
-    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsString(vm, processTargetNameUI));
+    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsNontrivialString(vm, processTargetNameUI));
     RETURN_IF_EXCEPTION(scope, JSValueMakeUndefined(context));
 #if ENABLE(GPU_PROCESS)
-    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsString(vm, processTargetNameGPU));
+    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsNontrivialString(vm, processTargetNameGPU));
     RETURN_IF_EXCEPTION(scope, JSValueMakeUndefined(context));
 #endif
-    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsString(vm, processTargetNameNetworking));
+    processTargetsObject->putDirectIndex(globalObject, index++, JSC::jsNontrivialString(vm, processTargetNameNetworking));
     RETURN_IF_EXCEPTION(scope, JSValueMakeUndefined(context));
     return toRef(vm, processTargetsObject);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to