Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(192)

Issue 1192743005: Reland 'Additional HandleScopes to limit Handle consumption.' (Closed)

Created:
5 years, 6 months ago by oth
Modified:
5 years, 6 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Reland 'Additional HandleScopes to limit Handle consumption.' v8 builds with --no-snap were hitting handle limits compiling natives for handle count unit tests that run with --check_handle_count. Patch now has higher handle limits (~4k more than required for failing natives compilation). Original issue: https://codereview.chromium.org/1185633002/ Original issue's description: > 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-t agcloud.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} BUG= Committed: https://crrev.com/af4c4b0427c44e74520d61f328f258b365ba233d Cr-Commit-Position: refs/heads/master@{#29322}

Patch Set 1 #

Patch Set 2 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1263 lines, -13 lines) Patch
M src/ast.cc View 1 1 chunk +3 lines, -1 line 0 comments Download
M src/handles.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M src/handles-inl.h View 1 chunk +13 lines, -1 line 0 comments Download
M src/heap/heap.cc View 1 2 chunks +4 lines, -2 lines 0 comments Download
M src/runtime/runtime-literals.cc View 1 chunk +1 line, -0 lines 0 comments Download
M test/cctest/test-types.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
A + test/mjsunit/handle-count-ast.js View 1 chunk +5 lines, -9 lines 0 comments Download
A test/mjsunit/handle-count-runtime-literals.js View 1 chunk +1230 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (9 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1192743005/1
5 years, 6 months ago (2015-06-22 10:11:14 UTC) #3
commit-bot: I haz the power
Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even ...
5 years, 6 months ago (2015-06-22 10:11:17 UTC) #5
oth
PTAL - attempting to reland handle count patch. This change just increases handle count limit ...
5 years, 6 months ago (2015-06-22 10:38:24 UTC) #8
rmcilroy
On 2015/06/22 10:38:24, orion wrote: > PTAL - attempting to reland handle count patch. This ...
5 years, 6 months ago (2015-06-26 08:27:28 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1192743005/20001
5 years, 6 months ago (2015-06-26 09:47:21 UTC) #12
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 6 months ago (2015-06-26 10:29:22 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1192743005/20001
5 years, 6 months ago (2015-06-26 10:56:12 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 6 months ago (2015-06-26 10:58:13 UTC) #17
commit-bot: I haz the power
5 years, 6 months ago (2015-06-26 10:58:33 UTC) #18
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/af4c4b0427c44e74520d61f328f258b365ba233d
Cr-Commit-Position: refs/heads/master@{#29322}

Powered by Google App Engine
This is Rietveld 408576698