[Compiler] Enable use of seperate zones for parsing and compiling.

[Compiler] Enable use of seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which can be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203
Review-Url: https://codereview.chromium.org/2645403002
Cr-Commit-Position: refs/heads/master@{#43089}
Committed: https://chromium.googlesource.com/v8/v8/+/1fc93f2e2d33aaa0424901d0187a2548bc5051fc

[rmcilroy, 2017-01-23 15:46:44]

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

[commit-bot: I haz the power, 2017-01-23 15:46:46]

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

[rmcilroy, 2017-01-23 15:49:51]

Patchset #1 (id:1) has been deleted

[rmcilroy, 2017-01-23 15:50:10]

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

[commit-bot: I haz the power, 2017-01-23 15:50:16]

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

[rmcilroy, 2017-01-23 15:54:34]

rmcilroy@chromium.org changed reviewers:
+ mstarzinger@chromium.org, verwaest@chromium.org

[rmcilroy, 2017-01-23 15:54:35]

mstarzinger@chromium.org: Please review changes in *compiler*

verwaest@chromium.org: Please review changes in zone/ and crankshaft/

https://codereview.chromium.org/2645403002/diff/20001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/2645403002/diff/20001/src/compiler.cc#newcode562
src/compiler.cc:562: !InnerFunctionIsAsmModule(&inner_literals)) {
I'm going to send out a separate mail to figure out how we can remove the need
for this asmjs check (it's because asm-wasm-builder.cc parses function
declarations itself during compilation), however for now this shouldn't affect
us since the compiler-dispatcher refuses to compile asm modules.

[commit-bot: I haz the power, 2017-01-23 16:22:49]

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

[commit-bot: I haz the power, 2017-01-23 16:22:50]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2017-01-26 21:24:47]

https://codereview.chromium.org/2645403002/diff/20001/src/crankshaft/hydrogen.h
File src/crankshaft/hydrogen.h (right):

https://codereview.chromium.org/2645403002/diff/20001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.h:41: compile_zone_(function->GetIsolate()->allocator(),
ZONE_NAME),
Is there value to creating separate zones if we can't parallelize anyway?
Allocating zones incurs runtime and memory overhead.

[rmcilroy, 2017-02-07 12:41:37]

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

[commit-bot: I haz the power, 2017-02-07 12:41:43]

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

[rmcilroy, 2017-02-07 12:41:53]

Comment addressed, PTAL, thanks.

https://codereview.chromium.org/2645403002/diff/20001/src/crankshaft/hydrogen.h
File src/crankshaft/hydrogen.h (right):

https://codereview.chromium.org/2645403002/diff/20001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.h:41: compile_zone_(function->GetIsolate()->allocator(),
ZONE_NAME),
On 2017/01/26 21:24:47, Toon Verwaest wrote:
> Is there value to creating separate zones if we can't parallelize anyway?
> Allocating zones incurs runtime and memory overhead.

Changed to only seperate zones when required and reuse the parse zone otherwise.

[commit-bot: I haz the power, 2017-02-07 12:46:54]

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

[commit-bot: I haz the power, 2017-02-07 12:46:55]

Dry run: Try jobs failed on following builders:
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/17000)
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/22149)

[rmcilroy, 2017-02-07 13:42:32]

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

[commit-bot: I haz the power, 2017-02-07 13:42:39]

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

[rmcilroy, 2017-02-07 13:43:00]

Description was changed from

==========
[Compiler] Use seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which should be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203, v8:5215
==========

to

==========
[Compiler] Use seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which should be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203
==========

[rmcilroy, 2017-02-07 13:57:04]

Description was changed from

==========
[Compiler] Use seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which should be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203
==========

to

==========
[Compiler] Enable use of seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which can be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203
==========

[commit-bot: I haz the power, 2017-02-07 14:06:33]

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

[commit-bot: I haz the power, 2017-02-07 14:06:34]

Dry run: This issue passed the CQ dry run.

[rmcilroy, 2017-02-08 17:28:54]

Ping?

[Michael Starzinger, 2017-02-09 14:23:31]

LGTM from my end.

https://codereview.chromium.org/2645403002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/2645403002/diff/60001/src/compiler.cc#newcode...
src/compiler.cc:1275: DCHECK(function->shared()->is_compiled());
Nice! Glad this finally holds by now.

https://codereview.chromium.org/2645403002/diff/60001/src/crankshaft/hydrogen.h
File src/crankshaft/hydrogen.h (left):

https://codereview.chromium.org/2645403002/diff/60001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.h:19: #include "src/zone/zone.h"
nit: Isn't this header needed for all the embedded ZoneList fields?

[rmcilroy, 2017-02-09 15:03:04]

Toon, could you review for Crankshaft please?

https://codereview.chromium.org/2645403002/diff/60001/src/compiler.cc
File src/compiler.cc (right):

https://codereview.chromium.org/2645403002/diff/60001/src/compiler.cc#newcode...
src/compiler.cc:1275: DCHECK(function->shared()->is_compiled());
On 2017/02/09 14:23:30, Michael Starzinger wrote:
> Nice! Glad this finally holds by now.

Acknowledged :).

https://codereview.chromium.org/2645403002/diff/60001/src/crankshaft/hydrogen.h
File src/crankshaft/hydrogen.h (left):

https://codereview.chromium.org/2645403002/diff/60001/src/crankshaft/hydrogen...
src/crankshaft/hydrogen.h:19: #include "src/zone/zone.h"
On 2017/02/09 14:23:30, Michael Starzinger wrote:
> nit: Isn't this header needed for all the embedded ZoneList fields?

Yes sorry missed this. Done.

[Toon Verwaest, 2017-02-10 08:30:57]

lgtm

[rmcilroy, 2017-02-10 09:28:29]

The CQ bit was checked by rmcilroy@chromium.org

[rmcilroy, 2017-02-10 09:28:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/2645403002/#ps80001
(title: "Add back header")

[commit-bot: I haz the power, 2017-02-10 09:28:41]

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, 2017-02-10 09:55:23]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1486718909573320,
"parent_rev": "7ef8cb562a4ecac0f28fe8308412cd14a770064f", "commit_rev":
"1fc93f2e2d33aaa0424901d0187a2548bc5051fc"}

[commit-bot: I haz the power, 2017-02-10 09:55:28]

Description was changed from

==========
[Compiler] Enable use of seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which can be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203
==========

to

==========
[Compiler] Enable use of seperate zones for parsing and compiling.

In order to allow parallel compilation of eager inner functions, we need to
seperate the zone used for parsing (which will be shared between all the
parallel compile jobs) and the zone used for compilation. This CL changes
CompilationInfo to require a zone (which can be different from the zone in
ParseInfo). We then seal the ParseInfo zone after parsing and analysis is done
to prevent any further allocation in that zone, so that it can be shared
(read-only) with the parallel compile jobs.

BUG=v8:5203

Review-Url: https://codereview.chromium.org/2645403002
Cr-Commit-Position: refs/heads/master@{#43089}
Committed:
https://chromium.googlesource.com/v8/v8/+/1fc93f2e2d33aaa0424901d0187a2548bc5...
==========

[commit-bot: I haz the power, 2017-02-10 09:55:29]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/1fc93f2e2d33aaa0424901d0187a2548bc5...