Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(11)

Issue 2775503006: [builtins] Improve performance of array.prototype.filter and map (Closed)

Created:
2 years, 2 months ago by mvstanton
Modified:
2 years, 1 month ago
Reviewers:
danno, Yang, adamk
CC:
v8-reviews_googlegroups.com, Michael Hablich, jgruber
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

[builtins] Improve performance of array.prototype.filter and map. BUG= Review-Url: https://codereview.chromium.org/2775503006 Cr-Commit-Position: refs/heads/master@{#44793} Committed: https://chromium.googlesource.com/v8/v8/+/1eb0ef316103caf526f9ab80290b5ba313e232af

Patch Set 1 #

Patch Set 2 : Small fix. #

Patch Set 3 : Some refactoring. #

Patch Set 4 : Rebase to include map. #

Patch Set 5 : Incorrect mode used. #

Patch Set 6 : REBASE. #

Patch Set 7 : compile fix. #

Patch Set 8 : Confusion with Parameter mode fix. #

Patch Set 9 : Remove old impl. #

Total comments: 10

Patch Set 10 : fixes #

Total comments: 6

Patch Set 11 : Fixes and rebase. #

Patch Set 12 : Comment response. #

Total comments: 2

Patch Set 13 : Code comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+269 lines, -150 lines) Patch
M src/bootstrapper.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +6 lines, -8 lines 0 comments Download
M src/builtins/builtins-array-gen.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +116 lines, -48 lines 0 comments Download
M src/code-stub-assembler.h View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +25 lines, -1 line 0 comments Download
M src/code-stub-assembler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +101 lines, -32 lines 0 comments Download
M src/js/array.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +0 lines, -54 lines 0 comments Download
M src/js/typedarray.js View 1 2 3 4 5 6 7 8 9 10 4 chunks +18 lines, -3 lines 0 comments Download
M test/cctest/test-code-stub-assembler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -3 lines 0 comments Download
M test/mjsunit/stack-traces.js View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 61 (45 generated)
mvstanton
Hi Danno, Here is the basic CL with filter and map performance improvements. I'll pull ...
2 years, 2 months ago (2017-03-24 13:31:12 UTC) #12
mvstanton
Hi Danno, Okay *now* it's actually ready, sorry for the bug..!
2 years, 2 months ago (2017-03-31 11:47:20 UTC) #18
mvstanton
Hi Danno, hi Yang: Yang, could you have a look at array.js and bootstrapper.cc? Danno, ...
2 years, 2 months ago (2017-04-03 13:39:26 UTC) #30
Yang
On 2017/04/03 13:39:26, mvstanton wrote: > Hi Danno, hi Yang: > > Yang, could you ...
2 years, 2 months ago (2017-04-03 13:43:11 UTC) #31
danno
Cool! Getting there. First round of feedback. https://codereview.chromium.org/2775503006/diff/160001/src/builtins/builtins-array-gen.cc File src/builtins/builtins-array-gen.cc (right): https://codereview.chromium.org/2775503006/diff/160001/src/builtins/builtins-array-gen.cc#newcode141 src/builtins/builtins-array-gen.cc:141: if (o_kind ...
2 years, 2 months ago (2017-04-03 15:35:28 UTC) #32
mvstanton
Hi Danno, here, comments addressed. https://codereview.chromium.org/2775503006/diff/160001/src/builtins/builtins-array-gen.cc File src/builtins/builtins-array-gen.cc (right): https://codereview.chromium.org/2775503006/diff/160001/src/builtins/builtins-array-gen.cc#newcode141 src/builtins/builtins-array-gen.cc:141: if (o_kind < DICTIONARY_ELEMENTS) ...
2 years, 2 months ago (2017-04-06 08:57:36 UTC) #35
jgruber
Got a couple of drive-by nits inline (but looks good to me otherwise): https://codereview.chromium.org/2775503006/diff/180001/src/code-stub-assembler.cc File ...
2 years, 2 months ago (2017-04-19 10:25:28 UTC) #38
mvstanton
Thanks Jakob, those were great comments! I finally dealt with confusion among SMI/INT_PTR representation under ...
2 years, 1 month ago (2017-04-23 09:00:09 UTC) #45
danno
lgtm with comment https://codereview.chromium.org/2775503006/diff/220001/src/code-stub-assembler.h File src/code-stub-assembler.h (right): https://codereview.chromium.org/2775503006/diff/220001/src/code-stub-assembler.h#newcode682 src/code-stub-assembler.h:682: // followed for allocation failure. Label ...
2 years, 1 month ago (2017-04-24 12:12:11 UTC) #48
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2775503006/240001
2 years, 1 month ago (2017-04-24 12:21:07 UTC) #52
mvstanton
Thanks! Addressed outdated comment...landin'.. https://codereview.chromium.org/2775503006/diff/220001/src/code-stub-assembler.h File src/code-stub-assembler.h (right): https://codereview.chromium.org/2775503006/diff/220001/src/code-stub-assembler.h#newcode682 src/code-stub-assembler.h:682: // followed for allocation failure. ...
2 years, 1 month ago (2017-04-24 12:21:16 UTC) #53
commit-bot: I haz the power
Committed patchset #13 (id:240001) as https://chromium.googlesource.com/v8/v8/+/1eb0ef316103caf526f9ab80290b5ba313e232af
2 years, 1 month ago (2017-04-24 12:47:33 UTC) #56
adamk
This CL seems to be causing failures on the arm64 nosnap builder: https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20nosnap%20-%20debug I can ...
2 years, 1 month ago (2017-04-24 22:48:01 UTC) #58
adamk
As a side-note, this CL could have used a more full-fledged description, and a link ...
2 years, 1 month ago (2017-04-24 22:50:32 UTC) #59
adamk
On 2017/04/24 22:48:01, adamk wrote: > This CL seems to be causing failures on the ...
2 years, 1 month ago (2017-04-24 23:16:46 UTC) #60
mvstanton
2 years, 1 month ago (2017-04-25 07:57:43 UTC) #61
Message was sent while issue was closed.
Sorry for the poor CL description! Normally I try to do better here...

Powered by Google App Engine
This is Rietveld 408576698