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

Issue 698981 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Workers being passed ArrayBuffers aren't GCed often enough

Project Member Reported by keishi@chromium.org, Mar 7 2017

Issue description

dsanders11 has created this repro case
https://jsfiddle.net/eak91tuh/2/

What seems to be happening is that the Worker is holding on to CrossThreadPersistents to the buffers in the main thread, but a gc isn't triggered in the Worker (because the allocation size is small), causing buffers to accumulate no matter how hard the main thread tries to GC them.

 
I can't see any CTPs being created for that testcase, the array buffer contents aren't on the Blink GC heap.

Is an/the issue here that SerializedScriptValue::dataLengthInBytes() doesn't account for transferable object sizes?
Yes, transferring an ArrayBuffer doesn't seem able to transfer the "external allocated memory" into the worker's context. Hence, v8 GC heuristics due to external allocations won't be activated.
I'm sorry for jumping in on bugs like this, but this one was a bit too interesting to pass up -- https://codereview.chromium.org/2734173002/
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 10 2017

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

commit c7711215a15658e8932087b6a9676859a89f7609
Author: sigbjornf <sigbjornf@opera.com>
Date: Fri Mar 10 06:29:37 2017

postMessage(): transfer allocation costs along with value.

A MessageEvent's data value will in some cases hold on to significant
amounts of off-heap memory, so we take care of registering that
external allocation with v8, so that it can be taken into consideration
by the GC triggering logic.

However, when posting a message to another context, we must arrange for
its total 'external allocation' to be associated with the target context.
Including the sizes of any transferables (array buffers), so balance the
books more accurately by also transferring the external allocation cost
of those transferables.

R=
BUG= 698981 

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

[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.h
[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/bindings/templates/methods.cpp.tmpl
[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/bindings/tests/results/core/V8TestObject.cpp
[modify] https://crrev.com/c7711215a15658e8932087b6a9676859a89f7609/third_party/WebKit/Source/wtf/typed_arrays/ArrayBufferContents.h

Comment 5 by sigbjo...@opera.com, Mar 10 2017

Cc: keishi@chromium.org
Components: -Blink>MemoryAllocator>GarbageCollection Blink>Bindings
Labels: -Pri-3 Pri-2
Owner: sigbjo...@opera.com
Status: Fixed (was: Available)
Summary: Workers being passed ArrayBuffers aren't GCed often enough (was: CrossThreadPersistents to large objects may cause bloat)

Sign in to add a comment