Bug 212954

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>