Add option to run ScopeIterator faster giving up nested scope chain.

Add option to run ScopeIterator faster giving up nested scope chain.

We'd like to be able to trade nested scope chain info (consisting of with, block and catch scopes) in favor of speed in some cases.

BUG=chromium:340285
LOG=N
R=ulan@chromium.org, pfeldman, ulan, yangguo

Committed: https://code.google.com/p/v8/source/detail?r=20162

[ulan, 2014-03-20 08:15:29]

Looks good, one comment:

https://codereview.chromium.org/203463011/diff/20001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/203463011/diff/20001/src/runtime.cc#newcode12263
src/runtime.cc:12263: // args[3]: boolean: fast bit
Could you give it a more descriptive name? Like ignore_nested_scopes or
function_scopes_only.

[aandrey, 2014-03-20 11:50:25]

https://codereview.chromium.org/203463011/diff/20001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/203463011/diff/20001/src/runtime.cc#newcode12263
src/runtime.cc:12263: // args[3]: boolean: fast bit
On 2014/03/20 08:15:29, ulan wrote:
> Could you give it a more descriptive name? Like ignore_nested_scopes or
> function_scopes_only.

Done.

[ulan, 2014-03-20 12:11:49]

One more place remaining:

https://codereview.chromium.org/203463011/diff/40001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/203463011/diff/40001/src/runtime.cc#newcode11737
src/runtime.cc:11737: bool fast = false)
fast => ignore_nested_scopes

[aandrey, 2014-03-20 13:09:05]

https://codereview.chromium.org/203463011/diff/40001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/203463011/diff/40001/src/runtime.cc#newcode11737
src/runtime.cc:11737: bool fast = false)
On 2014/03/20 12:11:49, ulan wrote:
> fast => ignore_nested_scopes

Done.

[ulan, 2014-03-20 13:13:08]

Thanks for fixing. Something seems to be wrong with the Patch Set 4: it changes
unrelated lines. Did you accidentally upload with incorrect base?

[aandrey, 2014-03-20 13:22:17]

On 2014/03/20 13:13:08, ulan wrote:
> Thanks for fixing. Something seems to be wrong with the Patch Set 4: it
changes
> unrelated lines. Did you accidentally upload with incorrect base?

Seems OK to me:
https://codereview.chromium.org/203463011/diff2/40001:60001/src/runtime.cc

[ulan, 2014-03-20 13:43:32]

On 2014/03/20 13:22:17, aandrey wrote:
> On 2014/03/20 13:13:08, ulan wrote:
> > Thanks for fixing. Something seems to be wrong with the Patch Set 4: it
> changes
> > unrelated lines. Did you accidentally upload with incorrect base?
> 
> Seems OK to me:
> https://codereview.chromium.org/203463011/diff2/40001:60001/src/runtime.cc

Yes, the diff between patch sets is OK, but the diff to the base is off so I
cannot apply your patch locally:
https://codereview.chromium.org/203463011/diff/60001/src/runtime.cc

https://codereview.chromium.org/download/issue203463011_60001.diff

This is probably because your base is git://github.com/v8/v8.git@master instead
of http://v8.googlecode.com/svn/branches/bleeding_edge/

Could you please try to rebase and reupload? If that doesn't work send me a raw
patch via email and I will upload a CL for you with the correct base.

For future CLs it is probably worth setting up your v8 checkout as described in
https://code.google.com/p/v8/wiki/Contributing

[aandrey, 2014-03-20 13:54:07]

On 2014/03/20 13:43:32, ulan wrote:
> On 2014/03/20 13:22:17, aandrey wrote:
> > On 2014/03/20 13:13:08, ulan wrote:
> > > Thanks for fixing. Something seems to be wrong with the Patch Set 4: it
> > changes
> > > unrelated lines. Did you accidentally upload with incorrect base?
> > 
> > Seems OK to me:
> > https://codereview.chromium.org/203463011/diff2/40001:60001/src/runtime.cc
> 
> Yes, the diff between patch sets is OK, but the diff to the base is off so I
> cannot apply your patch locally:
> https://codereview.chromium.org/203463011/diff/60001/src/runtime.cc
> 
> https://codereview.chromium.org/download/issue203463011_60001.diff
> 
> This is probably because your base is git://github.com/v8/v8.git@master
instead
> of http://v8.googlecode.com/svn/branches/bleeding_edge/
> 
> Could you please try to rebase and reupload? If that doesn't work send me a
raw
> patch via email and I will upload a CL for you with the correct base.
> 
> For future CLs it is probably worth setting up your v8 checkout as described
in
> https://code.google.com/p/v8/wiki/Contributing

Oh I see. Rebased.

[aandrey, 2014-03-20 14:08:08]

PTAL

[ulan, 2014-03-20 14:08:45]

lgtm

[aandrey, 2014-03-21 11:39:14]

PTAL.
Changed runtime.cc to not to add nested scope_info for GLOBAL_SCOPE (along with
EVAL_SCOPE), otherwise a debug test was failing.

if (scope_info->scope_type() != EVAL_SCOPE &&
    scope_info->scope_type() != GLOBAL_SCOPE) {
  nested_scope_chain_.Add(scope_info);
}

[ulan, 2014-03-21 12:02:21]

https://codereview.chromium.org/203463011/diff/110001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/203463011/diff/110001/src/runtime.cc#newcode1...
src/runtime.cc:11875: if (scope_info->scope_type() != EVAL_SCOPE &&
Considering the comment above: "we only provide the function scope", I think we
want this check here:
scope_info->scope_type() == FUNCTION_SCOPE

[ulan, 2014-03-21 12:31:06]

Committed patchset #8 manually as r20162 (presubmit successful).