Bug 78195 – [JSC] Cache the number of non-custom constructor arguments

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

If [ConstructorParameters=] is specified, JSC caches the number of constructor arguments for performance. However, at present, [ConstructorParameters=] is specified on a small part of custom constructors (It appears that people have forgotten to add [ConstructorParameters=]). Thus, we can improve JSC by the two changes:

[1] For non-custom constructors, CodeGeneratorJS.pm should cache the number of constructor arguments automatically without [ConstructorParameters=] (CodeGeneratorJS.pm can know the number of arguments by the [Constructor=...] signature).

[2] For custom constructors, we should add [ConstructorParameters=] to their IDL files. (Or, maybe we can completely remove [ConstructorParameters=] if the caching is not so important for practical performance. I am afraid that people will forget to add [ConstructorParameters=] in the future. Anyway let's discuss the topic in another bug.)

In this bug, we make a change for [1].

[Kentaro Hara]

Created attachment 126221 [details]
Patch

[Darin Adler]

Comment on attachment 126221 [details]
Patch

Looks good. I’d like to see some test covering this, though. It’s not great to land a fix like this without a test.

[Kentaro Hara]

Created attachment 127096 [details]
Patch

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 126221 [details])
> Looks good. I’d like to see some test covering this, though. It’s not great to land a fix like this without a test.

Darin: I added the tests. Would you take another look? Thanks!

[Darin Adler]

Comment on attachment 127096 [details]
Patch

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

> LayoutTests/fast/js/constructor-length.html:8
> +description("This tests the length property of constructors.");

Could we sort these alphabetically?

[Kentaro Hara]

Created attachment 127098 [details]
patch for commit

sorted tests alphabetically

[WebKit Review Bot]

Comment on attachment 127098 [details]
patch for commit

Clearing flags on attachment: 127098

Committed r107772: <http://trac.webkit.org/changeset/107772>