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

Issue 776273 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Memory overhead in JSArrayBuffer due to rarely used wasm metadata

Project Member Reported by mlippautz@chromium.org, Oct 19 2017

Issue description

Just realized this today while reviewing a CL related to object layout of JSArrayBuffer.

We currently waste 2 words for each JSArrayBuffer, and thus TypedArray, for a feature that is only ever going to be used rarely in wasm.

The fields I am thinking about:
  kAllocationBaseOffset
  kAllocationLengthOffset

For all TypedArrays these fields are wasted as backing_store and byte_length always define the range to free the backing store.

For wasm we already have two bits that should be enough to distinguish the buffers:
  class HasGuardRegion : public BitField<bool, 5, 1> {};
  class IsWasmBuffer : public BitField<bool, 6, 1> {};

We could dispatch the free call to wasm once those bits are set and free the backing store there, looking up allocation_base and allocation_length in a wasp-global data structure.
 
The problem to my eyes is that every buffer pays the price of the extra 2 pointers even when they aren't used - it would be good to be able to shift the space/time overhead so that only buffers that use a different allocation base need it.

Sounds like Michael's plan to let WASM decide on custom free-ing logic would work.

Comment 2 by eholk@chromium.org, Oct 19 2017

Having a Wasm structure that tracks this extra information sounds like a good idea.

One potential wrinkle is that these buffers can be externalized, which means we need to make sure we can get this information out to Blink as well. See Blink's version of ArrayBufferContents here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/wtf/typed_arrays/ArrayBufferContents.h?type=cs&q=ArrayBufferContents&sq=package:chromium&l=40

I doubt it'll be a huge problem, but we will need to take some extra care.

This also points out that we're frequently wasting two words on the Blink side as well. Maybe we can and should make a similar map there?

Comment 3 by eholk@chromium.org, Dec 15 2017

Owner: eholk@chromium.org
Status: Assigned (was: Available)

Comment 4 by eholk@chromium.org, Jan 5 2018

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 20 2018

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

commit f866af42ae5ef6472c18bc3b1e6d65452966e1cb
Author: Eric Holk <eholk@chromium.org>
Date: Tue Mar 20 17:59:38 2018

[wasm] Track Wasm allocations in WasmMemoryTracker

This moves the Wasm-specific metadata from being fields on the
ArrayBuffer into a table managed by WasmMemoryTracker.

Bug:  chromium:776273 
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Id8b050bfdfe0fbe9436fb055e92c08d503d3c2ba
Reviewed-on: https://chromium-review.googlesource.com/850550
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52080}
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/api.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/heap/array-buffer-tracker.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/objects-body-descriptors-inl.h
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/objects-printer.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/objects.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/objects/js-array-inl.h
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/objects/js-array.h
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/runtime/runtime-wasm.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/snapshot/deserializer.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/module-compiler.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/wasm-engine.h
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/wasm-memory.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/wasm-memory.h
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/wasm-module.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/src/wasm/wasm-objects.cc
[modify] https://crrev.com/f866af42ae5ef6472c18bc3b1e6d65452966e1cb/test/cctest/wasm/test-run-wasm-module.cc

Comment 6 by eholk@chromium.org, Mar 20 2018

Status: Fixed (was: Started)

Sign in to add a comment