Title: [274529] trunk/Source/WebCore
- Revision
- 274529
- Author
- shvaikal...@gmail.com
- Date
- 2021-03-16 16:02:23 -0700 (Tue, 16 Mar 2021)
Log Message
[WebIDL] Optimize convertRecord() to avoid double |key| lookup
https://bugs.webkit.org/show_bug.cgi?id=223219
Reviewed by Darin Adler.
This change replaces getOwnPropertyDescriptor() with more low-level API so
`slot.isTaintedByOpaqueObject()` can be leveraged to avoid the second |key|
lookup if it's unobservable, while still invoking Proxy's [[Get]] trap [1].
It's a common pattern used in JSC (see ObjectConstructor.cpp), which speeds up
convertRecord() by 20% (microbenchmark: 10 keys / 10k runs).
Also, this patch applies Geoffrey Garen's post-review feedback on r268852
by utilizing AddResult.
No new tests, no behavior change.
Test: imported/w3c/web-platform-tests/fetch/api/headers/headers-record.html
[1] https://heycam.github.io/webidl/#es-record (step 4.2.2)
* bindings/js/JSDOMConvertRecord.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (274528 => 274529)
--- trunk/Source/WebCore/ChangeLog 2021-03-16 23:00:57 UTC (rev 274528)
+++ trunk/Source/WebCore/ChangeLog 2021-03-16 23:02:23 UTC (rev 274529)
@@ -1,5 +1,30 @@
2021-03-16 Alexey Shvayka <shvaikal...@gmail.com>
+ [WebIDL] Optimize convertRecord() to avoid double |key| lookup
+ https://bugs.webkit.org/show_bug.cgi?id=223219
+
+ Reviewed by Darin Adler.
+
+ This change replaces getOwnPropertyDescriptor() with more low-level API so
+ `slot.isTaintedByOpaqueObject()` can be leveraged to avoid the second |key|
+ lookup if it's unobservable, while still invoking Proxy's [[Get]] trap [1].
+
+ It's a common pattern used in JSC (see ObjectConstructor.cpp), which speeds up
+ convertRecord() by 20% (microbenchmark: 10 keys / 10k runs).
+
+ Also, this patch applies Geoffrey Garen's post-review feedback on r268852
+ by utilizing AddResult.
+
+ No new tests, no behavior change.
+
+ Test: imported/w3c/web-platform-tests/fetch/api/headers/headers-record.html
+
+ [1] https://heycam.github.io/webidl/#es-record (step 4.2.2)
+
+ * bindings/js/JSDOMConvertRecord.h:
+
+2021-03-16 Alexey Shvayka <shvaikal...@gmail.com>
+
Cache cross-origin methods / accessors of Window and Location per lexical global object
https://bugs.webkit.org/show_bug.cgi?id=222739
Modified: trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h (274528 => 274529)
--- trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h 2021-03-16 23:00:57 UTC (rev 274528)
+++ trunk/Source/WebCore/bindings/js/JSDOMConvertRecord.h 2021-03-16 23:02:23 UTC (rev 274529)
@@ -107,8 +107,8 @@
// 5. Repeat, for each element key of keys in List order:
for (auto& key : keys) {
// 1. Let desc be ? O.[[GetOwnProperty]](key).
- JSC::PropertyDescriptor descriptor;
- bool didGetDescriptor = object->getOwnPropertyDescriptor(&lexicalGlobalObject, key, descriptor);
+ JSC::PropertySlot slot(object, JSC::PropertySlot::InternalMethodType::GetOwnProperty);
+ bool hasProperty = object->methodTable(vm)->getOwnPropertySlot(object, &lexicalGlobalObject, key, slot);
RETURN_IF_EXCEPTION(scope, { });
// 2. If desc is not undefined and desc.[[Enumerable]] is true:
@@ -115,13 +115,17 @@
// It's necessary to filter enumerable here rather than using DontEnumPropertiesMode::Exclude,
// to prevent an observable extra [[GetOwnProperty]] operation in the case of ProxyObject records.
- if (didGetDescriptor && descriptor.enumerable()) {
+ if (hasProperty && !(slot.attributes() & JSC::PropertyAttribute::DontEnum)) {
// 1. Let typedKey be key converted to an IDL value of type K.
auto typedKey = Detail::IdentifierConverter<K>::convert(lexicalGlobalObject, key);
RETURN_IF_EXCEPTION(scope, { });
// 2. Let value be ? Get(O, key).
- auto subValue = object->get(&lexicalGlobalObject, key);
+ JSC::JSValue subValue;
+ if (LIKELY(!slot.isTaintedByOpaqueObject()))
+ subValue = slot.getValue(&lexicalGlobalObject, key);
+ else
+ subValue = object->get(&lexicalGlobalObject, key);
RETURN_IF_EXCEPTION(scope, { });
// 3. Let typedValue be value converted to an IDL value of type V.
@@ -132,13 +136,12 @@
// Note: It's possible that typedKey is already in result if K is USVString and key contains unpaired surrogates.
if constexpr (std::is_same_v<K, IDLUSVString>) {
if (!typedKey.is8Bit()) {
- auto iterator = resultMap.find(typedKey);
- if (iterator != resultMap.end()) {
- ASSERT(result[iterator->value].key == typedKey);
- result[iterator->value].value = WTFMove(typedValue);
+ auto addResult = resultMap.add(typedKey, result.size());
+ if (!addResult.isNewEntry) {
+ ASSERT(result[addResult.iterator->value].key == typedKey);
+ result[addResult.iterator->value].value = WTFMove(typedValue);
continue;
}
- resultMap.add(typedKey, result.size());
}
} else
UNUSED_VARIABLE(resultMap);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
