Summary: | [JSC] Implement String.prototype.concat in JS builtins | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam, sam | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Yusuke Suzuki
2017-05-31 21:55:53 PDT
Created attachment 311683 [details]
Patch
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). 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. Committed r217648: <http://trac.webkit.org/changeset/217648> (In reply to Yusuke Suzuki from comment #4) > Committed r217648: <http://trac.webkit.org/changeset/217648> Need to rebaseline error messages. WIP... Committed r217649: <http://trac.webkit.org/changeset/217649> (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. 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. |