Bug 172798 – [JSC] Implement String.prototype.concat in JS builtins

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

[Yusuke Suzuki]

[JSC] Implement String.prototype.concat in JS builtins

[Yusuke Suzuki]

Created attachment 311683 [details]
Patch

[Sam Weinig]

Comment on attachment 311683 [details]
Patch

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

> Source/JavaScriptCore/builtins/StringPrototype.js:311
> +function stringConcatSlowPath()
> +{
> +    "use strict";

Is using arguments faster than using rest parameters? (I asked Saam recently and he was on the fence I think, or maybe just hadn't benchmarked it).

[Yusuke Suzuki]

Comment on attachment 311683 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/builtins/StringPrototype.js:311
>> +    "use strict";
> 
> Is using arguments faster than using rest parameters? (I asked Saam recently and he was on the fence I think, or maybe just hadn't benchmarked it).

One stupid problem is that our builtin JS system currently cannot accept default and rest parameters :(
We should extend our builtin JS generators (python scripts) to accept these forms.
It should be easy & simple task.

But, even if we do that, for builtin JS, I think we can introduce & use much better one: extending `@argument()` to accept arbitrary runtime integers instead of constant integers.
By doing so, we can avoid allocation for arguments / rest parameter arrays completely for builtin JS code.
So, personally, rather than extending builtin JS generators, I think we should do this thing.

So, I think we do not need to use rest parameters & arguments in JS builtin code when we do the above optimization.

[Yusuke Suzuki]

Committed r217648: <http://trac.webkit.org/changeset/217648>

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #4)
> Committed r217648: <http://trac.webkit.org/changeset/217648>

Need to rebaseline error messages. WIP...

[Yusuke Suzuki]

Committed r217649: <http://trac.webkit.org/changeset/217649>

[Saam Barati]

(In reply to Yusuke Suzuki from comment #3)
> Comment on attachment 311683 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311683&action=review
> 
> Thanks!
> 
> >> Source/JavaScriptCore/builtins/StringPrototype.js:311
> >> +    "use strict";
> > 
> > Is using arguments faster than using rest parameters? (I asked Saam recently and he was on the fence I think, or maybe just hadn't benchmarked it).
> 
> One stupid problem is that our builtin JS system currently cannot accept
> default and rest parameters :(
> We should extend our builtin JS generators (python scripts) to accept these
> forms.
> It should be easy & simple task.
Out of curiosity, why does this break the script? Does it try to validate syntax somehow?

> 
> But, even if we do that, for builtin JS, I think we can introduce & use much
> better one: extending `@argument()` to accept arbitrary runtime integers
> instead of constant integers.
> By doing so, we can avoid allocation for arguments / rest parameter arrays
> completely for builtin JS code.
> So, personally, rather than extending builtin JS generators, I think we
> should do this thing.
I agree.

> 
> So, I think we do not need to use rest parameters & arguments in JS builtin
> code when we do the above optimization.

[Yusuke Suzuki]

Comment on attachment 311683 [details]
Patch

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

>>>> Source/JavaScriptCore/builtins/StringPrototype.js:311
>>>> +    "use strict";
>>> 
>>> Is using arguments faster than using rest parameters? (I asked Saam recently and he was on the fence I think, or maybe just hadn't benchmarked it).
>> 
>> One stupid problem is that our builtin JS system currently cannot accept default and rest parameters :(
>> We should extend our builtin JS generators (python scripts) to accept these forms.
>> It should be easy & simple task.
>> 
>> But, even if we do that, for builtin JS, I think we can introduce & use much better one: extending `@argument()` to accept arbitrary runtime integers instead of constant integers.
>> By doing so, we can avoid allocation for arguments / rest parameter arrays completely for builtin JS code.
>> So, personally, rather than extending builtin JS generators, I think we should do this thing.
>> 
>> So, I think we do not need to use rest parameters & arguments in JS builtin code when we do the above optimization.
> 
> Out of curiosity, why does this break the script? Does it try to validate syntax somehow?

It's just a missing regexp in builtin generator python. We should extend the existing regexp to accept parameters including default and rest params.