New issue
Advanced search Search tips

Issue 672027 link

Starred by 2 users
Status: Fixed
Owner:
rmcilroy@chromium.org
Closed: Jan 2017
Cc:
hablich@chromium.org
mythria@chromium.org
mstarzinger@chromium.org
u...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 670675



Sign in to add a comment

Ignition memory leak at http://moxielogic.org

Project Member Reported by u...@chromium.org, Dec 7 2016 
I extracted a test that reproduces an apparent memory leak with ignition (see crbug.com/670675) in d8 shell.

The test is attached and consists of ~300 chained .concat(array_literal) calls where each array_literal has about 10K elements.

When running out/x64.debug/d8 array-concat.js --ignition-staging, I get OOM crash at 2GB.

When running without ignition, the memory usage stays within 100MB.

Not sure if it is WAI and/or fixable. Assigning to Ross to make that decision.
array-concat.js
8.6 MB View Download

Comment 1 by u...@chromium.org, Dec 7 2016

Labels: M-57 M-56

Comment 2 by rmcilroy@chromium.org, Dec 8 2016

Cc: mythria@chromium.org
Labels: Proj-Ignition
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ae741d042c834d2296769e04c27d6b2d4f6e469b

commit ae741d042c834d2296769e04c27d6b2d4f6e469b
Author: rmcilroy <rmcilroy@chromium.org>
Date: Thu Dec 15 10:59:57 2016

[Interpreter] Allocate registers used as call arguments on-demand.

Allocate the registers used as arguments to a call on-demand after visiting the
argument (or reciever). This means that the visited expression can use registers
that would otherwise have been allocated for arguments which haven't been
visited yet.

The reason for doing this is to avoid keeping things live in registers
unecessarily for chained function calls, which avoids a memory leak for
functions which chain a large number of calls with large temporary arguments /
recievers.

BUG= chromium:672027 

Review-Url: https://codereview.chromium.org/2557173004
Cr-Commit-Position: refs/heads/master@{#41714}

[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/src/interpreter/bytecode-generator.h
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/src/interpreter/bytecode-register-allocator.h
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/src/interpreter/bytecode-register.h
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/ForOf.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/Generators.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/Modules.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/PropertyCall.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/SuperCallAndSpread.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/bytecode_expectations/TopLevelObjectLiterals.golden
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/cctest/interpreter/test-bytecode-generator.cc
[add] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/mjsunit/ignition/regress-672027.js
[modify] https://crrev.com/ae741d042c834d2296769e04c27d6b2d4f6e469b/test/unittests/interpreter/bytecode-register-allocator-unittest.cc

Comment 4 by rmcilroy@chromium.org, Dec 15 2016

Cc: hablich@chromium.org
Labels: Merge-Request-56
This needs merging into m56

Comment 5 by dimu@chromium.org, Dec 16 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 16 2016

Labels: merge-merged-5.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/57845f979120057a4b965b4b8b4f5d585151c9f7

commit 57845f979120057a4b965b4b8b4f5d585151c9f7
Author: Ross McIlroy <rmcilroy@chromium.org>
Date: Fri Dec 16 11:00:23 2016

Merged: [Interpreter] Allocate registers used as call arguments on-demand.

Revision: ae741d042c834d2296769e04c27d6b2d4f6e469b

BUG= chromium:672027 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=mythria@chromium.org

Review-Url: https://codereview.chromium.org/2578183002 .
Cr-Commit-Position: refs/branch-heads/5.6@{#56}
Cr-Branched-From: bdd3886218dfe76e8560eb8a18401942452ae859-refs/heads/5.6.326@{#1}
Cr-Branched-From: 879f6599eee6e1dfcbe9a24bf688b261c03e9558-refs/heads/master@{#41014}

[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/src/interpreter/bytecode-generator.cc
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/src/interpreter/bytecode-generator.h
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/src/interpreter/bytecode-register-allocator.h
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/src/interpreter/bytecode-register.h
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/bytecode_expectations/ForOf.golden
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/bytecode_expectations/Generators.golden
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/bytecode_expectations/Modules.golden
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/bytecode_expectations/PropertyCall.golden
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/bytecode_expectations/TopLevelObjectLiterals.golden
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/cctest/interpreter/test-bytecode-generator.cc
[add] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/mjsunit/ignition/regress-672027.js
[modify] https://crrev.com/57845f979120057a4b965b4b8b4f5d585151c9f7/test/unittests/interpreter/bytecode-register-allocator-unittest.cc
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 19 2016 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by sheriffbot@chromium.org, Dec 23 2016 

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by rmcilroy@chromium.org, Dec 23 2016

Labels: -Hotlist-Merge-Approved -M-57 -Merge-Approved-56

Comment 10 by rmcilroy@chromium.org, Jan 24 2017

Status: Fixed (was: Started)
►

Sign in to add a comment