[webkit-reviews] review granted: [Bug 212954] [JSC] Simplify get*PropertyNames() methods and EnumerationMode : [Attachment 416418] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Alexey Shvayka
<shvaikalesh at gmail.com>'s request for review:
Bug 212954: [JSC] Simplify get*PropertyNames() methods and EnumerationMode
https://bugs.webkit.org/show_bug.cgi?id=212954

Attachment 416418: Patch

https://bugs.webkit.org/attachment.cgi?id=416418&action=review




--- Comment #6 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 416418
  --> https://bugs.webkit.org/attachment.cgi?id=416418
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?