Object.getOwnPropertySymbols on large list takes very long
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jul 2015 19:31:16 +0000 (19:31 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jul 2015 19:31:16 +0000 (19:31 +0000)
commit0cb339160f06f4993c105b8a11688f2cca7a1649
tree8ae7df64ac955c3955803d220a80efd96bb708d8
parent47199185a352f1810fd69b5e4ed793ec5b0508f6
Object.getOwnPropertySymbols on large list takes very long
https://bugs.webkit.org/show_bug.cgi?id=146137

Reviewed by Mark Lam.

Source/JavaScriptCore:

Before this patch, Object.getOwnPropertySymbols collects all the names including strings.
And after it's done, filter the names to only retrieve the symbols.
But it's so time consuming if the given object is a large non-holed array since it has
many indexed properties and all the indexes have to be converted to uniqued_strings and
added to the collection of property names (though they may not be of the requested type
and will be filtered out later)

This patch introduces PropertyNameMode.
We leverage this mode in 2 places.

1. PropertyNameArray side
It is set in PropertyNameArray and it filters the incoming added identifiers based on the mode.
It ensures that PropertyNameArray doesn't become so large in the pathological case.
And it ensures that non-expected typed keys by the filter (Symbols or Strings) are never added
to the property name array collections.
However it does not solve the whole problem because the huge array still incurs the many
"indexed property to uniqued string" conversion and the large iteration before adding the keys
to the property name array.

2. getOwnPropertyNames side
So we can use the PropertyNameMode in the caller side (getOwnPropertyNames) as a **hint**.
When the large iteration may occur, the caller side can use the PropertyNameMode as a hint to
avoid the iteration.
But we cannot exclusively rely on these caller side checks because it would require that we
exhaustively add the checks to all custom implementations of getOwnPropertyNames as well.
This process requires manual inspection of many pieces of code, and is error prone. Instead,
we only apply the caller side check in a few strategic places where it is known to yield
performance benefits; and we rely on the filter in PropertyNameArray::add() to reject the wrong
types of properties for all other calls to PropertyNameArray::add().

In this patch, there's a concept in use that is not clear just from reading the code, and hence
should be documented here. When selecting the PropertyNameMode for the PropertyNameArray to be
instantiated, we apply the following logic:

1. Only JavaScriptCore code is aware of ES6 Symbols.
We can assume that pre-existing external code that interfaces JSC are only looking for string named properties. This includes:
    a. WebCore bindings
    b. Serializer bindings
    c. NPAPI bindings
    d. Objective C bindings
2. In JSC, code that compute object storage space needs to iterate both Symbol and String named properties. Hence, use PropertyNameMode::Both.
3. In JSC, ES6 APIs that work with Symbols should use PropertyNameMode::Symbols.
4. In JSC, ES6 APIs that work with String named properties should use PropertyNameMode::Strings.

* API/JSObjectRef.cpp:
(JSObjectCopyPropertyNames):
* bindings/ScriptValue.cpp:
(Deprecated::jsToInspectorValue):
* bytecode/ObjectAllocationProfile.h:
(JSC::ObjectAllocationProfile::possibleDefaultPropertyCount):
* runtime/EnumerationMode.h:
(JSC::EnumerationMode::EnumerationMode):
(JSC::EnumerationMode::includeSymbolProperties): Deleted.
* runtime/GenericArgumentsInlines.h:
(JSC::GenericArguments<Type>::getOwnPropertyNames):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::getOwnPropertyNames):
* runtime/JSLexicalEnvironment.cpp:
(JSC::JSLexicalEnvironment::getOwnNonIndexPropertyNames):
* runtime/JSONObject.cpp:
(JSC::Stringifier::Stringifier):
(JSC::Stringifier::Holder::appendNextProperty):
(JSC::Walker::walk):
* runtime/JSObject.cpp:
(JSC::JSObject::getOwnPropertyNames):
* runtime/JSPropertyNameEnumerator.cpp:
(JSC::JSPropertyNameEnumerator::create):
* runtime/JSPropertyNameEnumerator.h:
(JSC::propertyNameEnumerator):
* runtime/JSSymbolTableObject.cpp:
(JSC::JSSymbolTableObject::getOwnNonIndexPropertyNames):
* runtime/ObjectConstructor.cpp:
(JSC::objectConstructorGetOwnPropertyNames):
(JSC::objectConstructorGetOwnPropertySymbols):
(JSC::objectConstructorKeys):
(JSC::defineProperties):
(JSC::objectConstructorSeal):
(JSC::objectConstructorFreeze):
(JSC::objectConstructorIsSealed):
(JSC::objectConstructorIsFrozen):
* runtime/PropertyNameArray.h:
(JSC::PropertyNameArray::PropertyNameArray):
(JSC::PropertyNameArray::mode):
(JSC::PropertyNameArray::addKnownUnique):
(JSC::PropertyNameArray::add):
(JSC::PropertyNameArray::isUidMatchedToTypeMode):
(JSC::PropertyNameArray::includeSymbolProperties):
(JSC::PropertyNameArray::includeStringProperties):
* runtime/StringObject.cpp:
(JSC::StringObject::getOwnPropertyNames):
* runtime/Structure.cpp:
(JSC::Structure::getPropertyNamesFromStructure):

Source/WebCore:

* bindings/js/Dictionary.cpp:
(WebCore::Dictionary::getOwnPropertiesAsStringHashMap):
(WebCore::Dictionary::getOwnPropertyNames):
* bindings/js/SerializedScriptValue.cpp:
(WebCore::CloneSerializer::serialize):
* bridge/NP_jsobject.cpp:
(_NPN_Enumerate):

Source/WebKit/mac:

* Plugins/Hosted/NetscapePluginInstanceProxy.mm:
(WebKit::NetscapePluginInstanceProxy::enumerate):

Source/WebKit2:

* WebProcess/Plugins/Netscape/NPJSObject.cpp:
(WebKit::NPJSObject::enumerate):

LayoutTests:

* js/regress/object-get-own-property-symbols-on-large-array-expected.txt: Added.
* js/regress/object-get-own-property-symbols-on-large-array.html: Added.
* js/regress/script-tests/object-get-own-property-symbols-on-large-array.js: Added.
(trial):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@187355 268f45cc-cd09-0410-ab3c-d52691b4dbfc
29 files changed:
LayoutTests/ChangeLog
LayoutTests/js/regress/object-get-own-property-symbols-on-large-array-expected.txt [new file with mode: 0644]
LayoutTests/js/regress/object-get-own-property-symbols-on-large-array.html [new file with mode: 0644]
LayoutTests/js/regress/script-tests/object-get-own-property-symbols-on-large-array.js [new file with mode: 0644]
Source/JavaScriptCore/API/JSObjectRef.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bindings/ScriptValue.cpp
Source/JavaScriptCore/bytecode/ObjectAllocationProfile.h
Source/JavaScriptCore/runtime/EnumerationMode.h
Source/JavaScriptCore/runtime/GenericArgumentsInlines.h
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
Source/JavaScriptCore/runtime/JSLexicalEnvironment.cpp
Source/JavaScriptCore/runtime/JSONObject.cpp
Source/JavaScriptCore/runtime/JSObject.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h
Source/JavaScriptCore/runtime/JSSymbolTableObject.cpp
Source/JavaScriptCore/runtime/ObjectConstructor.cpp
Source/JavaScriptCore/runtime/PropertyNameArray.h
Source/JavaScriptCore/runtime/StringObject.cpp
Source/JavaScriptCore/runtime/Structure.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/Dictionary.cpp
Source/WebCore/bindings/js/SerializedScriptValue.cpp
Source/WebCore/bridge/NP_jsobject.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/NPJSObject.cpp