Additional HandleScopes to limit Handle consumption.

Additional HandleScopes to limit Handle consumption.

erikcorry@chromium.org suggested digging into v8 handle usage. Found potential scopes in ast.cc and runtime-literals.cc and added tests.

The runtime-literals.cc change reduces peak handles in imaging-darkroom.js from 1,282,610 to 428,218. The ast.cc change reduces the peak handles in string-tagcloud.js from 80,738 to 8,176.

No significant handle count issues found with major websites, but substantial savings on some benchmarks and demos:

Kraken's imaging-darkroom.js down from 1,282,610 to 428,218 due to runtime-literals.cc scope.
SunSpider's string-tagcloud.js down from 80,738 to 8.176 due to ast.cc

http://www.flohofwoe.net/demos/dragons_asmjs.html (738,906 -> 478,296)
http://www.flohofwoe.net/demos/instancing_asmjs.html (737,884 -> 477,274)
https://dl.dropboxusercontent.com/u/16662598/Ports/DOSBox-web/doom.html?engine=dosbox-growth.js (1,724,114 -> 1,087,408)
https://kripken.github.io/ammo.js/examples/new/ammo.html (175,784 -> 142,058)

BUG=

Committed: https://crrev.com/3a4c7538839186aa38910c66c986abb563f4ccd2
Cr-Commit-Position: refs/heads/master@{#29155}

[oth, 2015-06-12 10:23:32]

oth@chromium.org changed reviewers:
+ erikcorry@chromium.org, yangguo@chromium.org

[oth, 2015-06-12 10:23:33]

Yang, Erik please take a look at the changes here. Thanks!

[Erik Corry Chromium.org, 2015-06-15 09:42:58]

lgtm

https://codereview.chromium.org/1185633002/diff/20001/src/ast.cc
File src/ast.cc (right):

https://codereview.chromium.org/1185633002/diff/20001/src/ast.cc#newcode544
src/ast.cc:544: HandleScope scope(isolate);
Please add comment explaining why this can't be moved to the top of the for()
loop.

https://codereview.chromium.org/1185633002/diff/20001/test/mjsunit/handle-cou...
File test/mjsunit/handle-count-runtime-literals.js (right):

https://codereview.chromium.org/1185633002/diff/20001/test/mjsunit/handle-cou...
test/mjsunit/handle-count-runtime-literals.js:12:
replace(/"[^"\\\n\r]*"|good|bad|none|-?\d+(?:\.\d*)?(:?[eE][+\-]?\d+)?/g, ']').
80 column limit exceeded here.

[oth, 2015-06-15 13:34:03]

Thanks! Revised code per comments. Had to shift handle thresholds to be able to
get tests to fail if scopes not present which must be due to changes lower down.

https://codereview.chromium.org/1185633002/diff/20001/src/ast.cc
File src/ast.cc (right):

https://codereview.chromium.org/1185633002/diff/20001/src/ast.cc#newcode544
src/ast.cc:544: HandleScope scope(isolate);
On 2015/06/15 09:42:58, Erik Corry Chromium.org wrote:
> Please add comment explaining why this can't be moved to the top of the for()
> loop.

Done.

https://codereview.chromium.org/1185633002/diff/20001/test/mjsunit/handle-cou...
File test/mjsunit/handle-count-runtime-literals.js (right):

https://codereview.chromium.org/1185633002/diff/20001/test/mjsunit/handle-cou...
test/mjsunit/handle-count-runtime-literals.js:12:
replace(/"[^"\\\n\r]*"|good|bad|none|-?\d+(?:\.\d*)?(:?[eE][+\-]?\d+)?/g, ']').
On 2015/06/15 09:42:58, Erik Corry Chromium.org wrote:
> 80 column limit exceeded here.

Done.

[oth, 2015-06-19 09:56:16]

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

[oth, 2015-06-19 09:56:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
erikcorry@chromium.org
Link to the patchset: https://codereview.chromium.org/1185633002/#ps50001
(title: "Refine comment.")

[commit-bot: I haz the power, 2015-06-19 09:56:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185633002/50001

[Erik Corry Chromium.org, 2015-06-19 10:08:35]

lgtm

[commit-bot: I haz the power, 2015-06-19 10:40:40]

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

[commit-bot: I haz the power, 2015-06-19 10:40:42]

Dry run: This issue passed the CQ dry run.

[oth, 2015-06-19 12:42:30]

The CQ bit was checked by oth@chromium.org

[commit-bot: I haz the power, 2015-06-19 12:42:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185633002/50001

[commit-bot: I haz the power, 2015-06-19 12:56:06]

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

[commit-bot: I haz the power, 2015-06-19 12:56:07]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/3547)

[rmcilroy, 2015-06-19 15:14:41]

On 2015/06/19 12:56:07, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_presubmit on tryserver.v8 (JOB_FAILED,
> http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/3547)

RS LGTM for owners.

[oth, 2015-06-19 15:16:02]

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

[commit-bot: I haz the power, 2015-06-19 15:16:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185633002/50001

[commit-bot: I haz the power, 2015-06-19 15:18:07]

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

[commit-bot: I haz the power, 2015-06-19 15:18:09]

Dry run: This issue passed the CQ dry run.

[oth, 2015-06-19 16:01:44]

The CQ bit was checked by oth@chromium.org

[commit-bot: I haz the power, 2015-06-19 16:02:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1185633002/50001

[commit-bot: I haz the power, 2015-06-19 16:04:20]

Committed patchset #4 (id:50001)

[commit-bot: I haz the power, 2015-06-19 16:04:33]

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

[arv (Not doing code reviews), 2015-06-19 18:24:48]

A revert of this CL (patchset #4 id:50001) has been created in
https://codereview.chromium.org/1194873004/ by arv@chromium.org.

The reason for reverting is: Fails the following test

handle-count-ast
handle-count-runtime-...

on V8 Linux - nosnap - debug - 1

http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap%20-%20....

[adamk, 2015-06-19 18:25:10]

A revert of this CL (patchset #4 id:50001) has been created in
https://codereview.chromium.org/1192943002/ by adamk@chromium.org.

The reason for reverting is: CHECK failures in handles-inl.h on arm64 nosnap
debug:

http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20s....