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

Issue 1194873004: Revert of Additional HandleScopes to limit Handle consumption. (Closed)

Created:
5 years, 6 months ago by arv (Not doing code reviews)
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

Revert of Additional HandleScopes to limit Handle consumption. (patchset #4 id:50001 of https://codereview.chromium.org/1185633002/) Reason for revert: 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-%20debug%20-%201/builds/851/steps/Check/logs/stdio 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-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} TBR=yangguo@chromium.org,erikcorry@chromium.org,oth@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Committed: https://crrev.com/87afca31adb009128cbf0b0ed50777be16e407dc Cr-Commit-Position: refs/heads/master@{#29160}

Patch Set 1 #

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

Messages

Total messages: 4 (0 generated)
arv (Not doing code reviews)
Created Revert of Additional HandleScopes to limit Handle consumption.
5 years, 6 months ago (2015-06-19 18:24:48 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1194873004/1
5 years, 6 months ago (2015-06-19 18:25:14 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 6 months ago (2015-06-19 18:25:30 UTC) #3
commit-bot: I haz the power
5 years, 6 months ago (2015-06-19 18:25:43 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/87afca31adb009128cbf0b0ed50777be16e407dc
Cr-Commit-Position: refs/heads/master@{#29160}

Powered by Google App Engine
This is Rietveld 408576698