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

Issue 206283002: Increase the "local variables in a function" limit. (Closed)

Created:
6 years, 9 months ago by marja
Modified:
6 years, 9 months ago
CC:
v8-dev
Visibility:
Public.

Description

Increase the "local variables in a function" limit. Based on experiments, it seems like nothing goes wrong if the limit is upped, and local variables still resolve correctly. Notes: - It's unclear what the "correct" limit is and which part of the code needs it. - https://codereview.chromium.org/11099063/ bumped the limit the last time. - Test will follow in a separate CL. R=dcarney@chromium.org,rossberg@chromium.org,jkummerow@chromium.org BUG=

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -5 lines) Patch
M src/messages.js View 1 chunk +1 line, -1 line 0 comments Download
M src/parser.h View 1 chunk +2 lines, -1 line 0 comments Download
M test/mjsunit/limit-locals.js View 2 chunks +4 lines, -3 lines 1 comment Download

Messages

Total messages: 5 (0 generated)
marja
dcarney, rossberg, jkummerow, ptal rossberg, you bumped the variable limit the last time (jkummerow, you ...
6 years, 9 months ago (2014-03-20 10:26:36 UTC) #1
dcarney
lgtm
6 years, 9 months ago (2014-03-20 10:27:10 UTC) #2
Jakob Kummerow
lgtm https://codereview.chromium.org/206283002/diff/1/test/mjsunit/limit-locals.js File test/mjsunit/limit-locals.js (right): https://codereview.chromium.org/206283002/diff/1/test/mjsunit/limit-locals.js#newcode45 test/mjsunit/limit-locals.js:45: assertEquals("prefix 131071 suffix", function_with_n_locals(131071)); Feel free to drop ...
6 years, 9 months ago (2014-03-20 10:37:14 UTC) #3
marja
.... aand it has changed. The current idea is: the limit comes from variable index ...
6 years, 9 months ago (2014-03-20 10:43:21 UTC) #4
rossberg
6 years, 9 months ago (2014-03-20 10:45:56 UTC) #5
Message was sent while issue was closed.
On 2014/03/20 10:26:36, marja wrote:
> dcarney, rossberg, jkummerow, ptal
> 
> rossberg, you bumped the variable limit the last time (jkummerow, you
reviewed;
> see description for a link) do you know what the "correct" limit is (if there
is
> one) and what will go wrong if we have too many variable?

Bumping the limit was an emergency solution for GWT at the time, IIRC. I later
resisted requests to bump the limit even further, since there is no reason to
assume that there ever is a "correct" one -- and we cannot bump the limit each
time some user hits it again with even more complicated code. Instead, we need
to educate those users that there will always be limits, and that they need to
write their code generators such that they can cope.

Having huge numbers of local variables in a single function will lead to
excessive compile times (not all relevant algorithms are necessarily linear) and
memory consumption, so we have to draw the line somewhere.

Thus, I'm actually opposed to this change.

Powered by Google App Engine
This is Rietveld 408576698