New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 824443 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue v8:7621
Owner:
Closed: Sep 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 822266



Sign in to add a comment

Wasm should trigger a GC before failing because of OOM

Project Member Reported by clemensh@chromium.org, Mar 21 2018

Issue description

This code currently fails with OOM:

=======================================
function asm() {
  'use asm';
  function f() {}
  return f;
}
for (var i = 0; i < 50000; ++i) {
  asm();
}
=======================================

It spawns new wasm instances in a loop. Eventually (after ~15 seconds on my machine) it will crash with OOM.
Note that all instances die right after being created, so the GC can always deallocate all instances except for one.

In 822266 we made sure to call FatalProcessOutOfMemory whenever an allocation in the NativeModule fails.
What we should really do is give the GC a chance to get rid of garbage.

Is this what Platform::OnCriticalMemoryPressure() is for? Or is there a better callback?

 

Comment 1 by u...@chromium.org, Mar 22 2018

Does asm call AdjustAmountOfExternalAllocatedMemory? That should be sufficient for this case. If the function is called but GC is not triggered then, something is off with GC heuristics.
We do in at least two places, and I see that we even call Isolate::MemoryPressureNotification under certain circumstances: https://cs.chromium.org/chromium/src/v8/src/wasm/wasm-code-manager.cc?sq=package:chromium&dr&l=769

I was not aware of that. I will check exactly which code path is being taken before the OOM is reported.
Owner: mstarzinger@chromium.org
Status: Assigned (was: Available)
Michi is already looking into this.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 27 2018

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

commit 39fcb5402b5ce7d3a05f63e74ca4b0fabeb3bd88
Author: Michael Starzinger <mstarzinger@chromium.org>
Date: Tue Mar 27 13:36:46 2018

[wasm] Make {NativeModule::compiled_module} a phantom reference.

This reduces time it takes for the compiled module to be reclaimed. It
switches the reference in question from a weak reference with finalizer
to a phantom reference, because the finalizer was only clearing the
reference by now anyways.

R=ahaas@chromium.org
BUG= chromium:824443 

Change-Id: I51f0dbd487281184f82fd6c79fcf27514721b819
Reviewed-on: https://chromium-review.googlesource.com/978243
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52247}
[modify] https://crrev.com/39fcb5402b5ce7d3a05f63e74ca4b0fabeb3bd88/src/wasm/wasm-code-manager.cc
[modify] https://crrev.com/39fcb5402b5ce7d3a05f63e74ca4b0fabeb3bd88/src/wasm/wasm-code-manager.h
[modify] https://crrev.com/39fcb5402b5ce7d3a05f63e74ca4b0fabeb3bd88/src/wasm/wasm-objects.cc

This is fixed, right?
Status: Fixed (was: Assigned)
Owner: clemensh@chromium.org
Status: Started (was: Fixed)
This is not fixed, the original reproducer still fails with:
test.js:1: Linking failure in asm.js: Internal wasm failure: AsmJs::Instantiate: Out of memory: wasm memory

Looking into ways to fix this.
Cc: ahaas@chromium.org
 Issue v8:8143  has been merged into this issue.
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 10

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

commit b19637eb22e9ac51659034d2671385b67a431d5b
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Sep 10 10:21:27 2018

[wasm][cleanup] Remove dead method

R=mstarzinger@chromium.org

Bug: v8:8015,  chromium:824443 
Change-Id: I017e8bf9033fba1afdddcc5e3057860519dfc119
Reviewed-on: https://chromium-review.googlesource.com/1213210
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55748}
[modify] https://crrev.com/b19637eb22e9ac51659034d2671385b67a431d5b/src/objects/js-array-buffer.cc
[modify] https://crrev.com/b19637eb22e9ac51659034d2671385b67a431d5b/src/objects/js-array-buffer.h

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 10

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

commit 6549dffab64facc85daa50a1160b7a16b42a1b3e
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Sep 10 10:28:27 2018

[wasm] Remove limit on cmpxchg loop

If there are many workers and we are very unlucky, the cmpxchg loop can
in fact fail for more than 5 times. This CL removes this unneeded
limitation to avoid spurious failures.

R=mstarzinger@chromium.org

Bug:  chromium:824443 
Change-Id: I0a6adde1330c8a8389a42b36bf44e516fae8c574
Reviewed-on: https://chromium-review.googlesource.com/1213170
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55749}
[modify] https://crrev.com/6549dffab64facc85daa50a1160b7a16b42a1b3e/src/wasm/wasm-memory.cc

Mergedinto: v8:7621
Status: Duplicate (was: Started)
This is a dupe of v8:7621. The problem seems to be that the second GC does not synchronize on the release of all previously collected JSArrayBuffers.

Sign in to add a comment