Bug 121829 (optimizeArraySplice) - optimize js splice
Summary: optimize js splice
Status: UNCONFIRMED
Alias: optimizeArraySplice
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-24 02:36 PDT by Peng Xinchao
Modified: 2013-10-02 00:15 PDT (History)
5 users (show)

See Also:


Attachments
Array splice should using ArrayStore First. (1.13 KB, patch)
2013-09-24 02:40 PDT, Peng Xinchao
no flags Details | Formatted Diff | Diff
new patch (1.10 KB, patch)
2013-09-24 03:42 PDT, Peng Xinchao
no flags Details | Formatted Diff | Diff
make clear code (1.54 KB, patch)
2013-09-29 22:30 PDT, Peng Xinchao
sam: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Xinchao 2013-09-24 02:36:42 PDT
splice should separte two steps -- shift and unshift. I think "ShiftCountMode" is not necessity.
Comment 1 Peng Xinchao 2013-09-24 02:40:23 PDT
Created attachment 212443 [details]
Array splice should using ArrayStore First.
Comment 2 Peng Xinchao 2013-09-24 03:42:11 PDT
Created attachment 212446 [details]
new patch
Comment 3 Darin Adler 2013-09-24 09:16:53 PDT
Comment on attachment 212443 [details]
Array splice should using ArrayStore First.

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

> runtime/JSArray.h:100
> -        return shiftCountWithAnyIndexingType(exec, startIndex, count);
> +        if (!shiftCountForShift(exec, startIndex, count)) 
> +            return shiftCountWithAnyIndexingType(exec, startIndex, count);
> +        
> +
> +        return true;

For what it’s worth, the much clearer to write this is:

    return shiftCountForShift(exec, startIndex, count) || shiftCountWithAnyIndexingType(exec, startIndex, count);
Comment 4 Peng Xinchao 2013-09-29 22:30:11 PDT
Created attachment 212953 [details]
make clear code
Comment 5 Mark Hahnenberg 2013-09-30 14:55:59 PDT
I'm confused. What problem is this solving?
Comment 6 Sam Weinig 2013-10-02 00:15:17 PDT
Comment on attachment 212953 [details]
make clear code

r- due to lack of ChangeLog and explanation.