Bug 222539 – [JSC] Optimize getEnumerableLength

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] Rename getEnumerableLength to getEnumerableLengthTracingJSProxy, and remove indirect call of getEnumerableLength

[Yusuke Suzuki]

Created attachment 421802 [details]
Patch

[Yusuke Suzuki]

Created attachment 421803 [details]
Patch

[Yusuke Suzuki]

Created attachment 421804 [details]
Patch

[Yusuke Suzuki]

Created attachment 421805 [details]
Patch

[Yusuke Suzuki]

Created attachment 421806 [details]
Patch

[Yusuke Suzuki]

Created attachment 421807 [details]
Patch

[Alexey Shvayka]

Comment on attachment 421807 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421807&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Now getEnumerableLength is only overridden by JSProxy. And this is called in the critical path of propertyNameEnumerator.

1. I am pretty sure we can remove JSProxy handling from getEnumerableLength() because it won't hit fast path of for/in anyway: canAccessPropertiesQuicklyForEnumeration() will reject it due to overriden getOwnPropertyNames().
2. We can't optimize access to JSProxy's indices as they are handled in JSDOMWindow::getOwnPropertySlotByIndex().
3. For/in over window is super slow anyway, as it yield a lot of event handler properties etc, and doesn't seem like a case we should optimize for.
4. I believe we can optimize enumeration for StringObject / Arguments / TypedArray via getEnumerableLength() overrides, but can we (relatively easily) speed up indexed access for them?

[Yusuke Suzuki]

Comment on attachment 421807 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421807&action=review

>> Source/JavaScriptCore/ChangeLog:8
>> +        Now getEnumerableLength is only overridden by JSProxy. And this is called in the critical path of propertyNameEnumerator.
> 
> 1. I am pretty sure we can remove JSProxy handling from getEnumerableLength() because it won't hit fast path of for/in anyway: canAccessPropertiesQuicklyForEnumeration() will reject it due to overriden getOwnPropertyNames().
> 2. We can't optimize access to JSProxy's indices as they are handled in JSDOMWindow::getOwnPropertySlotByIndex().
> 3. For/in over window is super slow anyway, as it yield a lot of event handler properties etc, and doesn't seem like a case we should optimize for.
> 4. I believe we can optimize enumeration for StringObject / Arguments / TypedArray via getEnumerableLength() overrides, but can we (relatively easily) speed up indexed access for them?

Thanks,

1, 2, 3: Sounds good. I'll remove JSProxy chasing part.
4. I think they are rare enough, so we could support with TypeInfo flag in the future. For now, my plan is just keeping it as is (not optimized yet).

[Yusuke Suzuki]

Comment on attachment 421807 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=421807&action=review

>>> Source/JavaScriptCore/ChangeLog:8
>>> +        Now getEnumerableLength is only overridden by JSProxy. And this is called in the critical path of propertyNameEnumerator.
>> 
>> 1. I am pretty sure we can remove JSProxy handling from getEnumerableLength() because it won't hit fast path of for/in anyway: canAccessPropertiesQuicklyForEnumeration() will reject it due to overriden getOwnPropertyNames().
>> 2. We can't optimize access to JSProxy's indices as they are handled in JSDOMWindow::getOwnPropertySlotByIndex().
>> 3. For/in over window is super slow anyway, as it yield a lot of event handler properties etc, and doesn't seem like a case we should optimize for.
>> 4. I believe we can optimize enumeration for StringObject / Arguments / TypedArray via getEnumerableLength() overrides, but can we (relatively easily) speed up indexed access for them?
> 
> Thanks,
> 
> 1, 2, 3: Sounds good. I'll remove JSProxy chasing part.
> 4. I think they are rare enough, so we could support with TypeInfo flag in the future. For now, my plan is just keeping it as is (not optimized yet).

For typed-array case, we could just extend the support by using canGetIndexQuickly like feature.

[Yusuke Suzuki]

Created attachment 421988 [details]
Patch

[Yusuke Suzuki]

Created attachment 421992 [details]
Patch

[Alexey Shvayka]

(In reply to Yusuke Suzuki from comment #9)
> For typed-array case, we could just extend the support by using canGetIndexQuickly like feature.

Yeah, we don't need getEnumerableLength() in a method table for that.
Regarding other objects: we can't optimize indexed access for them via IndexedForInContext, only enumeration, which won't benefit much + they are rare.

Fast for/in for TypedArray can be tracked in https://bugs.webkit.org/show_bug.cgi?id=135033.

[EWS]

Committed r273766: <https://commits.webkit.org/r273766>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421992 [details].

[Radar WebKit Bug Importer]

<rdar://problem/74952502>