Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(660)

Issue 13649003: Fix worst-case behavior of MergeRemovableSimulates(). (Closed)

Created:
7 years, 8 months ago by Yang
Modified:
7 years, 8 months ago
Reviewers:
Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Fix worst-case behavior of MergeRemovableSimulates(). Currently, when a long series of removable simulates are merged, we do this by merging them one by one as we find them. As we merge the value value lists of the simulates, those lists snowball so that we get a quadratic complexity wrt runtime and memory consumption. Instead, we gather simulates that need to be merged, and merge them backwards starting from the last simulate. R=jkummerow@chromium.org BUG=v8:2612 Committed: https://code.google.com/p/v8/source/detail?r=14169

Patch Set 1 #

Total comments: 1

Patch Set 2 : small changes. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -42 lines) Patch
M src/hydrogen.cc View 1 2 chunks +22 lines, -18 lines 1 comment Download
M src/hydrogen-instructions.h View 1 chunk +1 line, -1 line 0 comments Download
M src/hydrogen-instructions.cc View 1 2 chunks +16 lines, -11 lines 0 comments Download
M test/mjsunit/mjsunit.status View 1 chunk +1 line, -0 lines 0 comments Download
A + test/mjsunit/regress/regress-2612.js View 1 chunk +42 lines, -12 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Yang
7 years, 8 months ago (2013-04-04 16:57:43 UTC) #1
Jakob Kummerow
LGTM. Thanks for fixing this! https://chromiumcodereview.appspot.com/13649003/diff/1/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right): https://chromiumcodereview.appspot.com/13649003/diff/1/src/hydrogen-instructions.cc#newcode2001 src/hydrogen-instructions.cc:2001: list->at(j)->DeleteAndReplaceWith(NULL); nit: s/list->at(j)/from/
7 years, 8 months ago (2013-04-04 17:12:22 UTC) #2
Yang
On 2013/04/04 17:12:22, Jakob wrote: > LGTM. Thanks for fixing this! > > https://chromiumcodereview.appspot.com/13649003/diff/1/src/hydrogen-instructions.cc > ...
7 years, 8 months ago (2013-04-05 08:11:38 UTC) #3
Jakob Kummerow
LGTM with a comment. https://chromiumcodereview.appspot.com/13649003/diff/5001/src/hydrogen.cc File src/hydrogen.cc (right): https://chromiumcodereview.appspot.com/13649003/diff/5001/src/hydrogen.cc#newcode3430 src/hydrogen.cc:3430: // Simply remove all accumulated ...
7 years, 8 months ago (2013-04-08 17:09:59 UTC) #4
Yang
7 years, 8 months ago (2013-04-08 17:37:32 UTC) #5
Message was sent while issue was closed.
Committed patchset #2 manually as r14169 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698