Replaced different means of zone pooling/reusing by one zone segment pool

Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409
Committed: https://crrev.com/a124feb0760896c8be61de08004a08c3bc9b4b3f
Committed: https://crrev.com/fc840361e357a571c709e0239ae82cc089800b3f
Committed: https://crrev.com/a96c2129af6203ffbf7512344e24f430fde82f29
Cr-Original-Original-Commit-Position: refs/heads/master@{#39633}
Cr-Original-Commit-Position: refs/heads/master@{#40048}
Cr-Commit-Position: refs/heads/master@{#40138}

[heimbuef, 2016-09-19 13:55:10]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

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

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

[heimbuef, 2016-09-19 13:57:02]

Patchset #1 (id:1) has been deleted

[heimbuef, 2016-09-19 13:57:11]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 13:57: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-19 14:03:49]

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

[commit-bot: I haz the power, 2016-09-19 14:03:50]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/12981)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[heimbuef, 2016-09-19 14:28:13]

Patchset #1 (id:20001) has been deleted

[heimbuef, 2016-09-19 14:28:24]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 14:28:29]

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-19 14:36:18]

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

[commit-bot: I haz the power, 2016-09-19 14:36:19]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/24361)

[heimbuef, 2016-09-19 15:02:58]

heimbuef@google.com changed reviewers:
+ mstarzinger@chromium.org

[heimbuef, 2016-09-19 15:02:58]

PTAL

[Michael Starzinger, 2016-09-20 10:35:39]

Cool stuff. Looking good. Just nits.

https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats.cc
File src/compiler/zonestats.cc (right):

https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats....
src/compiler/zonestats.cc:106: // Delete zone or clear and stash on unused_.
nit: Comment is outdated by now.

https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats.h
File src/compiler/zonestats.h (right):

https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats....
src/compiler/zonestats.h:5: #ifndef V8_COMPILER_zone_stats_H_
nit: All uppercase. Also, lets rename the file to zone-stats.h (with a hyphen)
for consistency.

https://codereview.chromium.org/2348303002/diff/40001/src/zone/zone.cc
File src/zone/zone.cc (right):

https://codereview.chromium.org/2348303002/diff/40001/src/zone/zone.cc#newcod...
src/zone/zone.cc:100: // and freeing every segment except the one we wish to
keep.
nit: Lets drop the "except the one we wish to keep" part from the comment.

https://codereview.chromium.org/2348303002/diff/40001/test/unittests/compiler...
File test/unittests/compiler/zone-pool-unittest.cc (right):

https://codereview.chromium.org/2348303002/diff/40001/test/unittests/compiler...
test/unittests/compiler/zone-pool-unittest.cc:13: class ZoneStatsTest : public
TestWithIsolate {
nit: The file should be renamed to zone-stats-unittest.cc now.

[heimbuef, 2016-09-20 12:08:42]

Description was changed from

==========
Replaced different means of zone pooling/reusing by one zone segment pool
BUG=
==========

to

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409
==========

[heimbuef, 2016-09-21 10:58:37]

Patchset #2 (id:60001) has been deleted

[heimbuef, 2016-09-21 10:58:56]

On 2016/09/20 at 10:35:39, mstarzinger wrote:
> Cool stuff. Looking good. Just nits.
> 
>
https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats.cc
> File src/compiler/zonestats.cc (right):
> 
>
https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats....
> src/compiler/zonestats.cc:106: // Delete zone or clear and stash on unused_.
> nit: Comment is outdated by now.
> 
> https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats.h
> File src/compiler/zonestats.h (right):
> 
>
https://codereview.chromium.org/2348303002/diff/40001/src/compiler/zonestats....
> src/compiler/zonestats.h:5: #ifndef V8_COMPILER_zone_stats_H_
> nit: All uppercase. Also, lets rename the file to zone-stats.h (with a hyphen)
for consistency.
> 
> https://codereview.chromium.org/2348303002/diff/40001/src/zone/zone.cc
> File src/zone/zone.cc (right):
> 
>
https://codereview.chromium.org/2348303002/diff/40001/src/zone/zone.cc#newcod...
> src/zone/zone.cc:100: // and freeing every segment except the one we wish to
keep.
> nit: Lets drop the "except the one we wish to keep" part from the comment.
> 
>
https://codereview.chromium.org/2348303002/diff/40001/test/unittests/compiler...
> File test/unittests/compiler/zone-pool-unittest.cc (right):
> 
>
https://codereview.chromium.org/2348303002/diff/40001/test/unittests/compiler...
> test/unittests/compiler/zone-pool-unittest.cc:13: class ZoneStatsTest : public
TestWithIsolate {
> nit: The file should be renamed to zone-stats-unittest.cc now.

All done.

[Toon Verwaest, 2016-09-21 11:31:02]

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

[Toon Verwaest, 2016-09-21 11:31:03]

https://codereview.chromium.org/2348303002/diff/80001/src/compiler/zone-stats.cc
File src/compiler/zone-stats.cc (right):

https://codereview.chromium.org/2348303002/diff/80001/src/compiler/zone-stats...
src/compiler/zone-stats.cc:89: used_.push_back(zone);
Does it still make sense to call this "used_" rather than just "zones_"?

https://codereview.chromium.org/2348303002/diff/80001/src/compiler/zone-stats...
src/compiler/zone-stats.cc:93: void ZoneStats::ReturnZone(Zone* zone) {
Does it make sense to call this Return, now that there's no pool?

https://codereview.chromium.org/2348303002/diff/80001/src/runtime/runtime-reg...
File src/runtime/runtime-regexp.cc (right):

https://codereview.chromium.org/2348303002/diff/80001/src/runtime/runtime-reg...
src/runtime/runtime-regexp.cc:477: Zone zone(isolate->allocator());
Iirc one of these runtime zones is actually nesting the outer zone. Shouldn't we
just pass in the existing zone in that case; creating a new one in the other
client? I'm not sure anymore which is which though.

[Michael Starzinger, 2016-09-21 11:53:29]

LGTM from my end with one last comment to address.

https://codereview.chromium.org/2348303002/diff/80001/test/unittests/compiler...
File test/unittests/compiler/zone-stats-unittest.cc (right):

https://codereview.chromium.org/2348303002/diff/80001/test/unittests/compiler...
test/unittests/compiler/zone-stats-unittest.cc:5: #include
<compiler/zone-stats.h>
Please use the #include "src/compiler/zone-stats.h" (with quotation marks
instead of angle brackets) syntax for non-system headers.

[heimbuef, 2016-09-22 14:46:49]

Patchset #3 (id:100001) has been deleted

[heimbuef, 2016-09-22 14:47:04]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-22 14:47:12]

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-22 15:22:05]

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

[commit-bot: I haz the power, 2016-09-22 15:22:06]

Dry run: This issue passed the CQ dry run.

[heimbuef, 2016-09-22 15:32:20]

The CQ bit was checked by heimbuef@google.com

[heimbuef, 2016-09-22 15:32:21]

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/2348303002/#ps120001
(title: "Merge branch 'zonesegpool' into onezonepool")

[commit-bot: I haz the power, 2016-09-22 15:32:26]

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-22 15:35:03]

Committed patchset #3 (id:120001)

[commit-bot: I haz the power, 2016-09-22 15:35:19]

Description was changed from

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409
==========

to

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409

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

[commit-bot: I haz the power, 2016-09-22 15:35:21]

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

[Michael Hablich, 2016-09-23 06:02:00]

A revert of this CL (patchset #3 id:120001) has been created in
https://codereview.chromium.org/2360403003/ by hablich@chromium.org.

The reason for reverting is: Blocks Roll
https://codereview.chromium.org/2366733002/.

[heimbuef, 2016-10-06 09:33:19]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-06 09:33:24]

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-10-06 10:03:50]

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

[commit-bot: I haz the power, 2016-10-06 10:03:52]

Dry run: This issue passed the CQ dry run.

[heimbuef, 2016-10-06 11:48:00]

Please take another look - just fixed the blink build.

[Michael Starzinger, 2016-10-06 14:34:16]

Yep, still LGTM. Thanks!

[heimbuef, 2016-10-06 14:48:05]

The CQ bit was checked by heimbuef@google.com

[commit-bot: I haz the power, 2016-10-06 14:48:12]

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-10-06 15:16:33]

Committed patchset #5 (id:160001)

[commit-bot: I haz the power, 2016-10-06 15:16:51]

Description was changed from

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409

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

to

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409

Committed: https://crrev.com/a124feb0760896c8be61de08004a08c3bc9b4b3f
Committed: https://crrev.com/fc840361e357a571c709e0239ae82cc089800b3f
Cr-Original-Commit-Position: refs/heads/master@{#39633}
Cr-Commit-Position: refs/heads/master@{#40048}
==========

[commit-bot: I haz the power, 2016-10-06 15:16:52]

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

[Michael Hablich, 2016-10-07 21:10:30]

A revert of this CL (patchset #5 id:160001) has been created in
https://codereview.chromium.org/2401163002/ by hablich@chromium.org.

The reason for reverting is: related to roll blocker:
https://codereview.chromium.org/2400343002/.

[heimbuef, 2016-10-10 18:10:02]

The CQ bit was checked by heimbuef@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 18:10:14]

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-10-10 18:38:44]

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

[commit-bot: I haz the power, 2016-10-10 18:38:45]

Dry run: This issue passed the CQ dry run.

[heimbuef, 2016-10-10 18:57:50]

The CQ bit was checked by heimbuef@google.com

[commit-bot: I haz the power, 2016-10-10 18:57:55]

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-10-10 19:00:37]

Committed patchset #5 (id:160001)

[commit-bot: I haz the power, 2016-10-10 19:01:04]

Description was changed from

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409

Committed: https://crrev.com/a124feb0760896c8be61de08004a08c3bc9b4b3f
Committed: https://crrev.com/fc840361e357a571c709e0239ae82cc089800b3f
Cr-Original-Commit-Position: refs/heads/master@{#39633}
Cr-Commit-Position: refs/heads/master@{#40048}
==========

to

==========
Replaced different means of zone pooling/reusing by one zone segment pool

BUG=v8:5409

Committed: https://crrev.com/a124feb0760896c8be61de08004a08c3bc9b4b3f
Committed: https://crrev.com/fc840361e357a571c709e0239ae82cc089800b3f
Committed: https://crrev.com/a96c2129af6203ffbf7512344e24f430fde82f29
Cr-Original-Original-Commit-Position: refs/heads/master@{#39633}
Cr-Original-Commit-Position: refs/heads/master@{#40048}
Cr-Commit-Position: refs/heads/master@{#40138}
==========

[commit-bot: I haz the power, 2016-10-10 19:01:06]

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