Bug 121829 (optimizeArraySplice)

Summary: optimize js splice
Product: WebKit Reporter: Peng Xinchao <xinchao.peng>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Array splice should using ArrayStore First.
none
new patch
none
make clear code sam: review-

Peng Xinchao
Reported 2013-09-24 02:36:42 PDT
splice should separte two steps -- shift and unshift. I think "ShiftCountMode" is not necessity.
Attachments
Array splice should using ArrayStore First. (1.13 KB, patch)
2013-09-24 02:40 PDT, Peng Xinchao 		no flags
DetailsFormatted DiffDiff
new patch (1.10 KB, patch)
2013-09-24 03:42 PDT, Peng Xinchao 		no flags
DetailsFormatted DiffDiff
make clear code (1.54 KB, patch)
2013-09-29 22:30 PDT, Peng Xinchao 		sam: review-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Peng Xinchao
Comment 1 2013-09-24 02:40:23 PDT
Created attachment 212443 [details] Array splice should using ArrayStore First.
Peng Xinchao
Comment 2 2013-09-24 03:42:11 PDT
Created attachment 212446 [details] new patch
Darin Adler
Comment 3 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);
Peng Xinchao
Comment 4 2013-09-29 22:30:11 PDT
Created attachment 212953 [details] make clear code
Mark Hahnenberg
Comment 5 2013-09-30 14:55:59 PDT
I'm confused. What problem is this solving?
Sam Weinig
Comment 6 2013-10-02 00:15:17 PDT
Comment on attachment 212953 [details] make clear code r- due to lack of ChangeLog and explanation.
Note You need to log in before you can comment on or make changes to this bug.