[Parser] Don't internalize on-the-fly.

[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953
Committed: https://crrev.com/a06df1f21c583801844c4b19f92217beb458c22f
Cr-Commit-Position: refs/heads/master@{#39531}

[rmcilroy, 2016-09-09 15:15:35]

rmcilroy@chromium.org changed reviewers:
+ ahaas@chromium.org, marja@chromium.org

[rmcilroy, 2016-09-09 15:15:36]

Marja, PTAL. This is based on your original CL, but doesn't require isolate to
be stored in Parser (due to https://codereview.chromium.org/2324803002/). This
also depends on Jochen's scope chain changes
(https://codereview.chromium.org/2306413002/).

ahaas@, PTAL at asmjs typer changes.

[marja, 2016-09-12 07:21:48]

Cool!

lgtm % question

The description is promising a bit much though. Because of the rewriter,
internalization doesn't happen at a single defined point :(

https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc
File src/parsing/rewriter.cc (right):

https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc#new...
src/parsing/rewriter.cc:350:
info->ast_value_factory()->Internalize(info->isolate());
Is this needed? Isn't the name internalized at this point anyway?

[ahaas, 2016-09-12 07:46:34]

On 2016/09/12 at 07:21:48, marja wrote:
> Cool!
> 
> lgtm % question
> 
> The description is promising a bit much though. Because of the rewriter,
internalization doesn't happen at a single defined point :(
> 
> https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc
> File src/parsing/rewriter.cc (right):
> 
>
https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc#new...
> src/parsing/rewriter.cc:350:
info->ast_value_factory()->Internalize(info->isolate());
> Is this needed? Isn't the name internalized at this point anyway?

asmjs lgtm

[rmcilroy, 2016-09-12 12:20:10]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 12:20:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-12 12:49:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-12 12:49:23]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-09-16 09:19:14]

The CQ bit was checked by rmcilroy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 09:19:18]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 09:44:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 09:44:39]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2016-09-19 13:24:19]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-09-19 13:24:19]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2328593002/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-19 13:24:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rmcilroy, 2016-09-19 13:25:59]

The CQ bit was unchecked by rmcilroy@chromium.org

[rmcilroy, 2016-09-19 13:26:31]

Description was changed from

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.
It also makes Internalization only happen at a single well defined point,

BUG=v8:5215, chromium:634953
==========

to

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953
==========

[Toon Verwaest, 2016-09-20 08:39:21]

verwaest@chromium.org changed reviewers:
+ verwaest@chromium.org

[Toon Verwaest, 2016-09-20 08:39:23]

I approve!

[rmcilroy, 2016-09-20 08:50:46]

> The description is promising a bit much though. Because of the rewriter,
> internalization doesn't happen at a single defined point :(

Removed the "single defined point" text.

https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc
File src/parsing/rewriter.cc (right):

https://codereview.chromium.org/2328593002/diff/1/src/parsing/rewriter.cc#new...
src/parsing/rewriter.cc:350:
info->ast_value_factory()->Internalize(info->isolate());
On 2016/09/12 07:21:48, marja wrote:
> Is this needed? Isn't the name internalized at this point anyway?

Yeah this is needed. The "dot_result_string" needs to be internalized before
running the processor, and it isn't at this point unless something in the parser
referenced it.

[rmcilroy, 2016-09-20 09:00:00]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2016-09-20 09:00:01]

The patchset sent to the CQ was uploaded after l-g-t-m from ahaas@chromium.org,
marja@chromium.org
Link to the patchset: https://codereview.chromium.org/2328593002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-09-20 09:00:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-20 09:32:23]

Description was changed from

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953
==========

to

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953
==========

[commit-bot: I haz the power, 2016-09-20 09:32:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-20 09:33:11]

Description was changed from

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953
==========

to

==========
[Parser] Don't internalize on-the-fly.

Avoid internalizing on-the-fly now that scope analysis and natives syntax
runtime calls no longer require internalized AST values. This should be
more efficient by avoiding extra branches on every AST value creation.

BUG=v8:5215, chromium:634953

Committed: https://crrev.com/a06df1f21c583801844c4b19f92217beb458c22f
Cr-Commit-Position: refs/heads/master@{#39531}
==========

[commit-bot: I haz the power, 2016-09-20 09:33:12]

Patchset 4 (id:??) landed as
https://crrev.com/a06df1f21c583801844c4b19f92217beb458c22f
Cr-Commit-Position: refs/heads/master@{#39531}