Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External memory leak on node v8.x #21021

Closed
cirias opened this issue May 30, 2018 · 4 comments
Closed

External memory leak on node v8.x #21021

cirias opened this issue May 30, 2018 · 4 comments
Assignees
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@cirias
Copy link

cirias commented May 30, 2018

  • Version: 8.11.2
  • Platform: Linux sirius-Alienware-15-R3 4.13.0-41-generic Tracking / Assuring Compatibility #46~16.04.1-Ubuntu SMP Thu May 3 10:06:43 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

The code below will cause external memory leak of v8 engine. In v8, mark-compact updates the external_memory before the gc, which cause the external_memory_limit too big. Normally, It's fine, we just got a bigger limit. But when scavenge happened before next mc, updated the external_memory, the external_memory_limit will keep increasing.

I thought this issue has already been fixed in these commits, and node v9.x already ported them:
https://codereview.chromium.org/2917853004
https://codereview.chromium.org/2921883002

function triggerScavenge() {
  let arr = [];
  for (let i = 0; i < 5000; i++) {
    arr.push({});
  }

  setTimeout(triggerScavenge, 50);
}

let ds = [];

function triggerMarkCompact() {
  const { rss, heapTotal, heapUsed, external } = process.memoryUsage();
  console.log('------memoryUsage------', rss, heapTotal, heapUsed, external);

  for (let i = 0; i < 1000; i++) {
    ds.push(new ArrayBuffer(1024));
  }

  if (ds.length > 40000) {
    ds = [];
  }

  setTimeout(triggerMarkCompact, 200);
}

triggerScavenge();
triggerMarkCompact();

Run the script with --trace-gc to dump gc info.

Attached a graph of the memory usage
memory-leak

@Trott Trott added the v8 engine Issues and PRs related to the V8 dependency. label May 31, 2018
@Trott
Copy link
Member

Trott commented May 31, 2018

@nodejs/v8

@ofrobots ofrobots self-assigned this May 31, 2018
@ofrobots
Copy link
Contributor

The fix came to v9.x in the form of an upgrade to V8 6.1 on #14730.

It needs to be individually back-ported for 8.x. Self-assigning to do this / investigate feasibility once I have some bandwidth, but if anyone else has cycles sooner, please go ahead and re-assign.

ofrobots added a commit to ofrobots/node that referenced this issue Jun 11, 2018
Original commit message:
  [heap] Activate memory reducer on external memory activity.

  BUG=chromium:728228,chromium:626082
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

Refs: nodejs#21021
ofrobots added a commit to ofrobots/node that referenced this issue Jun 11, 2018
Original commit message:
  [heap] Lower external allocation limit when external memory shrinks.

  BUG=chromium:728228
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

Fixes: nodejs#21021
@ofrobots
Copy link
Contributor

Backport on #21269.

MylesBorins pushed a commit that referenced this issue Jun 14, 2018
Original commit message:
  [heap] Activate memory reducer on external memory activity.

  BUG=chromium:728228,chromium:626082
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

PR-URL: #21269
Fixes: #21021
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 14, 2018
Original commit message:
  [heap] Lower external allocation limit when external memory shrinks.

  BUG=chromium:728228
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

PR-URL: #21269
Fixes: #21021
Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg pushed a commit that referenced this issue Aug 16, 2018
Original commit message:
  [heap] Activate memory reducer on external memory activity.

  BUG=chromium:728228,chromium:626082
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

PR-URL: #21269
Fixes: #21021
Reviewed-By: Myles Borins <myles.borins@gmail.com>
rvagg pushed a commit that referenced this issue Aug 16, 2018
Original commit message:
  [heap] Lower external allocation limit when external memory shrinks.

  BUG=chromium:728228
  CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_rel_ng

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

PR-URL: #21269
Fixes: #21021
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@ofrobots
Copy link
Contributor

ofrobots commented Sep 6, 2018

The fix has been merged on 8.x and will be released as part of 8.12.0. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

3 participants