Changeset 185899 in webkit for trunk/Source/JavaScriptCore/runtime/JSString.h

Timestamp:

Jun 23, 2015, 7:33:18 PM (10 years ago)

Author:

Darin Adler

Message:

Make Array.join work directly on substrings without reifying them
​https://bugs.webkit.org/show_bug.cgi?id=146191

Reviewed by Andreas Kling.

Source/JavaScriptCore:

Besides the Array.join change, this has other optimizations based on
profiling the Peacekeeper array benchmark.

I measured a 14% speed improvement in the Peacekeeper array benchmark.

Still a lot of low hanging fruit in that test because so many of functions
on the array prototype are not optimizing for simple cases. For example,
the reverse function does individual get and put calls even when the array
is entirely made up of integers in contiguous storage.

• runtime/ArrayPrototype.cpp:

(JSC::getProperty): Use tryGetIndexQuickly first before getPropertySlot.
(JSC::argumentClampedIndexFromStartOrEnd): Marked inline.
(JSC::shift): Use the getProperty helper in this file instead of using
getPropertySlot. Use putByIndexInline instead of calling putByIndex directly.
In both cases this can yield a faster code path.
(JSC::unshift): Ditto.
(JSC::arrayProtoFuncToString): Updated to use the new JSStringJoiner
interface. Changed local variable name to thisArray since it's not a
JSObject*. Changed loop index to i instead of k.
(JSC::arrayProtoFuncToLocaleString): Updated to use the new JSStringJoiner
interface. Renamed thisObj to thisObject. Added a missing exception check
after the toLocaleString function is called, but before toString is called
the result of that function.
(JSC::arrayProtoFuncJoin): Updated to use the new JSStringJointer interface.
Added a missing exception check after calling toString on the separator
but before calling get to get the first element in the array-like object
being joined. Changed loop index to i instead of k. Added missing exception
check after calling toString on each string from the array before calling
get for the next element.
(JSC::arrayProtoFuncConcat): Use JSArray::length instead of using the
getLength function.
(JSC::arrayProtoFuncReverse): Ditto. Also use putByIndexInline.
(JSC::arrayProtoFuncShift): Ditto.
(JSC::arrayProtoFuncSplice): Use getIndex instead of get, which includes some
additional optimizations.
(JSC::getOrHole): Deleted. Unused function.
(JSC::arrayProtoFuncUnShift): Use putByIndexInline.

• runtime/ExceptionHelpers.cpp:

(JSC::errorDescriptionForValue): Removed the duplicate copy of the the logic
from JSValue::toString.

• runtime/JSCJSValue.cpp:

(JSC::JSValue::toStringSlowCase): Improved the performance when converting a
small integer to a single character string.
(JSC::JSValue::toWTFStringSlowCase): Moved the contents of the
inlineJSValueNotStringtoString function here.

• runtime/JSCJSValue.h: Removed no longer used toWTFStringInline and fixed

a comment with a typo.

• runtime/JSObject.h:

(JSC::JSObject::putByIndexInline): Marked ALWAYS_INLINE because this was not
getting inlined at some call sites.
(JSC::JSObject::indexingData): Deleted. Unused function.
(JSC::JSObject::currentIndexingData): Deleted. Unused function.
(JSC::JSObject::getHolyIndexQuickly): Deleted. Unused function.
(JSC::JSObject::relevantLength): Deleted. Unused function.
(JSC::JSObject::currentRelevantLength): Deleted. Unused function.

• runtime/JSString.h: Added the StringViewWithUnderlyingString struct and

the viewWithUnderlyingString function. Removed the inlineJSValueNotStringtoString
and toWTFStringInline functions.

• runtime/JSStringJoiner.cpp:

(JSC::appendStringToData): Changed this to be a template instead of writing
it out, since StringView::getCharactersWithUpconvert does almsot exactly what
this function was trying to do.
(JSC::joinStrings): Rewrote this to use StringView.
(JSC::JSStringJoiner::joinedLength): Added. Factored out from the join function.
(JSC::JSStringJoiner::join): Rewrote to make it a bit simpler. Added an assertion
that we entirely filled capacity, since we are now reserving capacity and using
uncheckedAppend. Use String instead of RefPtr<StringImpl> because there was no
particular value to using the impl directly.

• runtime/JSStringJoiner.h: Changed the interface to the class to use StringView.

Also changed this class so it now has the responsibility to convert each JSValue
into a string. This let us share more code between toString and join, and also
lets us use the new viewWithUnderlyingString function, which could be confusing at
all the call sites, but is easier to understand here.

Source/WTF:

• wtf/Vector.h: Added an overload of uncheckedAppend like the one we added

a while back, a non-template function that forwards through to the function
template. This lets us call uncheckedAppend on an argument list and have it
properly convert it to the Vector's element type.

• wtf/text/StringView.h:

(WTF::StringView::getCharactersWithUpconvert): Changed to not use memcpy;
saw some indication the hand-written loop was faster when profiling. Also
use m_length directly when we know we are dealing with an 8-bit string,
since the masking that the index function does is not needed in that case.
(WTF::StringView::UpconvertedCharacters::UpconvertedCharacters): Ditto.
(WTF::StringView::toString): Ditto.
(WTF::StringView::toFloat): Ditto.
(WTF::StringView::toInt): Ditto.
(WTF::StringView::toStringWithoutCopying): Ditto.
(WTF::StringView::find): Ditto.

File:

• trunk/Source/JavaScriptCore/runtime/JSString.h (modified) (6 diffs)

trunk/Source/JavaScriptCore/runtime/JSString.h

@@ -63 +63 @@
 JSRopeString* jsStringBuilder(VM*);
 
+struct StringViewWithUnderlyingString {
+    StringView view;
+    String underlyingString;
+};
+
 class JSString : public JSCell {
 public:
@@ -145 +150 @@
     RefPtr<AtomicStringImpl> toExistingAtomicString(ExecState*) const;
     StringView view(ExecState*) const;
+    StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
     const String& value(ExecState*) const;
     const String& tryGetValue() const;
@@ -385 +391 @@
     void clearFibers() const;
     StringView view(ExecState*) const;
+    StringViewWithUnderlyingString viewWithUnderlyingString(ExecState&) const;
 
     WriteBarrierBase<JSString>& fiber(unsigned i) const
@@ -674 +681 @@
 }
 
-ALWAYS_INLINE String inlineJSValueNotStringtoString(const JSValue& value, ExecState* exec)
-{
-    VM& vm = exec->vm();
-    if (value.isInt32())
-        return vm.numericStrings.add(value.asInt32());
-    if (value.isDouble())
-        return vm.numericStrings.add(value.asDouble());
-    if (value.isTrue())
-        return vm.propertyNames->trueKeyword.string();
-    if (value.isFalse())
-        return vm.propertyNames->falseKeyword.string();
-    if (value.isNull())
-        return vm.propertyNames->nullKeyword.string();
-    if (value.isUndefined())
-        return vm.propertyNames->undefinedKeyword.string();
-    return value.toString(exec)->value(exec);
-}
-
-ALWAYS_INLINE String JSValue::toWTFStringInline(ExecState* exec) const
-{
-    if (isString())
-        return static_cast<JSString*>(asCell())->value(exec);
-
-    return inlineJSValueNotStringtoString(*this, exec);
-}
-
 ALWAYS_INLINE StringView JSRopeString::view(ExecState* exec) const
 {
@@ -711 +692 @@
 }
 
+ALWAYS_INLINE StringViewWithUnderlyingString JSRopeString::viewWithUnderlyingString(ExecState& state) const
+{
+    if (isSubstring()) {
+        auto& base = substringBase()->m_value;
+        if (is8Bit())
+            return { { base.characters8() + substringOffset(), m_length }, base };
+        return { { base.characters16() + substringOffset(), m_length }, base };
+    }
+    resolveRope(&state);
+    return { m_value, m_value };
+}
+
 ALWAYS_INLINE StringView JSString::view(ExecState* exec) const
 {
@@ -718 +711 @@
 }
 
+ALWAYS_INLINE StringViewWithUnderlyingString JSString::viewWithUnderlyingString(ExecState& state) const
+{
+    if (isRope())
+        return static_cast<const JSRopeString&>(*this).viewWithUnderlyingString(state);
+    return { m_value, m_value };
+}
+
 inline bool JSString::isSubstring() const
 {