JSAPIValueWrapper doesn't override any of the forms of getPropertyNames. Hence, there probably is no reason to have this flag, but we should investigate deeper before removing it. If the flag can be remove, then we can strengthen the assertion for overridesAnyFormOfGetPropertyNames() in Structure::validateFlags().
Ditto for JSGlobalObject. We should investigate whether the flag is really needed.
Closed the wrong bug. Re-opening.
Created attachment 416300 [details] Patch
(In reply to Alexey Shvayka from comment #3) > Created attachment 416300 [details] > Patch r270393 patch object-get-own-property-symbols 64.6840+-0.9261 ^ 61.2901+-0.6377 ^ definitely 1.0554x faster object-keys-cloned-arguments 64.2298+-0.9984 ^ 52.0452+-1.3066 ^ definitely 1.2341x faster object-keys-error-object 392.5292+-9.7568 ^ 77.6232+-1.5012 ^ definitely 5.0569x faster
Created attachment 416418 [details] Patch Fix early return condition in getNonReifiedStaticPropertyNames() and export getOwnSpecialPropertyNames().
Comment on attachment 416418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=416418&action=review r=me > Source/JavaScriptCore/runtime/JSObject.cpp:2384 > +constexpr unsigned maximumPrototypeChainDepth = 40000; Let's put `static ` before this. And maybe, defining it in JSObject's class static field (JSObject::maximumPrototypeChainDepth) is better. > Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.cpp:110 > + // Inlined JSObject::getOwnNonIndexPropertyNames() > + base->methodTable(vm)->getOwnSpecialPropertyNames(base, globalObject, propertyNames, DontEnumPropertiesMode::Exclude); > + RETURN_IF_EXCEPTION(scope, void()); > + > + base->getNonReifiedStaticPropertyNames(vm, propertyNames, DontEnumPropertiesMode::Exclude); > + unsigned nonStructurePropertyCount = propertyNames.size(); > + structure->getPropertyNamesFromStructure(vm, propertyNames, DontEnumPropertiesMode::Exclude); > + scope.assertNoException(); > + > + // |propertyNames| contains properties exclusively from the structure. > + if (!nonStructurePropertyCount) > + structurePropertyCount = propertyNames.size(); This means that, if non-reified property exists, we cannot use fast path (since nonStructurePropertyCount is non-zero, and structurePropertyCount is always zero). Is it the same in the old code? If the old code can use the fast path even if non-reified static property exists, can you change the new code to the same state?
Created attachment 416886 [details] Patch Use JSObject::maximumPrototypeChainDepth and add a test case for non-reified static properties.
(In reply to Yusuke Suzuki from comment #6) > This means that, if non-reified property exists, we cannot use fast path > (since nonStructurePropertyCount is non-zero, and structurePropertyCount is > always zero). > Is it the same in the old code? If the old code can use the fast path even > if non-reified static property exists, can you change the new code to the > same state? Yes, right, this is very intentional improvement over old code: to match the order required by the spec, structure properties should come after non-reified static properties, which isn't possible on the fast path because structure loop comes before generic loop. This change is performance-neutral because slow path is taken only if there is an [[Enumerable]] non-reified or special property, which is extremely rare. In JSC, only `WebAssembly` and `WebAssembly.Module` objects have [[Enumerable]] non-reified properties. However, even they, as well as WebIDL interfaces like `Element.prototype`, never passed the canAccessPropertiesQuicklyForEnumeration() check because of accessors. I've added a test for for/in with `WebAssembly`, which used to fail before https://bugs.webkit.org/show_bug.cgi?id=214908.
Created attachment 416888 [details] Patch Guard stress test with `WebAssembly` check.
Tools/Scripts/svn-apply failed to apply attachment 416888 [details] to trunk. Please resolve the conflicts and upload a new patch.
Committed r271269: <https://trac.webkit.org/changeset/271269>
<rdar://problem/72908707>