Best way to avoid copying buffers between WebKit <-> chromium IPC

[Alec Flett]

For some IDB performance work, I'm trying to reduce buffer copies but I'm getting lost in the abstractions.

I've got to send buffers two directions between webkit and chromium, including webkit code that lives in the browser process.. which means I need to optimize 4 different areas 

WebCore -> chromium WebKit::* -> chromium IPC

chromium IPC -> chromium WebKit::* -> WebCore

What I *had* been doing (ugly, I know, hurray for types) is just:

WTF::Vector<uint8_t> -> WebKit::WebVector<uint8> -> std::vector<unsigned char>

and vice versa.

I'm using std::vector in my IPC messages, mostly out of convention - it doesn't look like anyone is using the Web* classes in messages... except for a few clever uses of IPC_STRUCT_TRAITS*

But of course each of these is a copy. I looked into adding some "adoption" methods to WebVector, but it gets ugly fast

What I'm looking at now is using WebCore::SharedBuffer so that I have:

WebCore::SharedBuffer::adoptVector(data) -> WebKit::WebData -> std::vector

and vice versa.

this avoids the WTF::Vector <-> WebKit::WebVector copying. Before I venture further (like trying to figure out how to put WebKit::WebData in an IPC message without copying the buffer) I'm curious:

1) Am I headed down the wrong path here? Are there better abstractions in WebKit that I should be using?

2) Surely I'm not the first to do this.. I see lots of buffers and uses of WebData/SharedBuffer all over chromium WebKit, but none of it seems particularly straight forward (i.e. they're one-off cases to load some resource or something)

3) If I am heading down the right path, anyone have any pointers to good example uses of IPC_STRUCT_TRAITS_* to describe a buffer?

Alec

[Adam Barth]

We should be able to make WebVector be able to adopt the underlying
storage from a WTF::Vector (assuming you're not using any inline
capacity) by teaching WebVector to call WTF::Vector::releaseBuffer.
I'm not sure if there's a way to get the memory into a std::vector
without copying, but at least that will save you one copy.

Adam

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

[Adam Barth]

Maybe you looked into this possibility already?

Is there more to it than:

WebVector(WTF::Vector<T>& other)
{
m_size = other.size();
m_ptr = m_size ? other.releaseBuffer() : 0;
}

?

Adam

[Darin Fisher]

In cases where we want to cut out all buffer copying across a sequence of IPC and API boundaries, we generally use shared memory.  You can see this happen for resource loading, painting, and some other image data cases.

In this particular case, is the data purely temporary?  Or, does the receiver need to hold onto the data for a while?

-Darin

On Fri, Feb 1, 2013 at 11:38 AM, Alec Flett <alec...@chromium.org> wrote:

--

[Alec Flett]

Re: Adam's suggestion of WebVector(WTF::Vector<T>& other) - I did try this and I think it could be this simple but I wasn't sure about introducing a dependency at that layer (I'm still trying to figure out what layers are not allowed to overlap... WebData seems to be more specifically tuned for this but I'm open to suggestions.

Re: shared memory - well the calls are all async, and on the render->browser side of things we use the memory past the lifetime of the incoming IPC call.. but in both directions, the reciever can take ownership of the memory. Where/what are the shared memory primitives and to they allow for this kind of transfer of ownership without a huge amount of synchronization? 

[Adam Barth]

Take a look at WebPrivatePtr.h for example:

#if WEBKIT_IMPLEMENTATION
#include <wtf/PassRefPtr.h>
#endif

The important thing is just to keep it behind the WEBKIT_IMPLEMENTATION ifdef.

Adam

[Antoine Labour]

On Sat, Feb 2, 2013 at 9:47 PM, Darin Fisher <da...@chromium.org> wrote:

In cases where we want to cut out all buffer copying across a sequence of IPC and API boundaries, we generally use shared memory.  You can see this happen for resource loading, painting, and some other image data cases.

Beware that on many platforms, shared memory have a cost - sometimes hidden - when creating/destroying as well as mapping/unmapping, that far outweighs the cost of an extra copy. It makes it that shared memory is mostly a win when:

- you have to do these transfers often

- you can keep the shared memory created/mapped over a lifetime much longer than the transfer itself, and re-use it across many transfers.

Sending a new shared memory with every transfer is usually a loss unless the data is very large.

Antoine

[Alec Flett]

I guess my question is, is there anything wrong with using WebKit::WebData and WebCore::SharedBuffer, or would improving WebVector be more desirable for the platform? (I've got a working implementation with WebData)

Alec

[Adam Barth]

WebData and SharedBuffer seem like they're much too complicated for
what you want. They do all sorts of fancy things involving reference
counting, etc. If you add this constructor to WebVector, I'm sure
lots of other code will use it as well. :)

Adam

[Alec Flett]

ok, so I realized it's one thing to adopt a Vector using Vector::releaseBuffer(), but I also need to go the other way (have WTF::Vector adopt a WebVector's buffer)

I can't see a way to do this right now without something like:

WebVector::releaseToVector() {
  Vector v;
  Vector v.release(m_size);
  *(v.dataSlot()) = data.relaseBuffer()
  return v;
}

(relying on some compiler optimization, but you get the idea)

Mostly I'm wary of calling Vector::dataSlot() because there are no callers right now.

Another option would be to put this into vector:
    <typename U>
    void adopt(U& other) {
        m_size = other.size();
        m_data = other.releaseBuffer();
    }

(adding releaseBuffer() to WebKVector too)

Thoughts?

On Feb 4, 2013 5:38 PM, "Alec Flett" <alec...@google.com> wrote:

ok, so I realized it's one thing to adopt a Vector using Vector::releaseBuffer(), but I also need to go the other way (have WTF::Vector adopt a WebVector's buffer)

I can't see a way to do this right now without something like:

WebVector::releaseToVector() {

  Vector v;

  Vector v.release(m_size);

  *(v.dataSlot()) = data.relaseBuffer()

  return v;

}

(relying on some compiler optimization, but you get the idea)

Mostly I'm wary of calling Vector::dataSlot() because there are no callers right now.

Another option would be to put this into vector:

    <typename U>

    void adopt(U& other) {

        m_size = other.size();

        m_data = other.releaseBuffer();

    }

(adding releaseBuffer() to WebKVector too)

Thoughts?

[Adam Barth]

I would try the

> <typename U>
> void adopt(U& other) {

approach.

Adam

On Mon, Feb 4, 2013 at 5:38 PM, Alec Flett <alec...@google.com> wrote:
>
> ok, so I realized it's one thing to adopt a Vector using
> Vector::releaseBuffer(), but I also need to go the other way (have
> WTF::Vector adopt a WebVector's buffer)
>
> I can't see a way to do this right now without something like:
>
> WebVector::releaseToVector() {
> Vector v;
> Vector v.release(m_size);
> *(v.dataSlot()) = data.relaseBuffer()
> return v;
> }
>
> (relying on some compiler optimization, but you get the idea)
>
> Mostly I'm wary of calling Vector::dataSlot() because there are no callers
> right now.
>
> Another option would be to put this into vector:
> <typename U>
> void adopt(U& other) {
> m_size = other.size();
> m_data = other.releaseBuffer();
> }
>
> (adding releaseBuffer() to WebKVector too)
>
> Thoughts?
>
>

[Alec Flett]

ok, so I'm starting to enter a twisty maze of templates.

What I've got so far does implement Vector::adopt but now I'm hitting the fact that WTF::Vector uses WTF::fastMalloc whereas WebKit::WebVector uses new/delete. So when I release a buffer from a WebVector into a Vector, all hell breaks loose. (I'm also just discovering that FastMalloc is a fork of tcmalloc...)

I can't just make WebVector fast-allocated because then the constructors + destructors will depend on WTF... if I go with WEBKIT_IMPLEMENTATION on that then we run the risk that if you create a WebVector on the chromium side and pass it to some WebKit:: chromium code to destroy it there, it will blow up... I could just make WebVector start using malloc/free directly, but it's unclear to me if this will call the underlying tcmalloc implementation or not.

I'm at a bit of a loss and starting to lean more towards WebCore::SharedBuffer

[Adam Barth]

Yeah, I hadn't thought of the fastMalloc issue. That means the copy
is doing something useful (in that it's copying from one heap to
another). It sounds like SharedBuffer might be a better approach.

Adam

[Albert J. Wong (王重傑)]

crazy idea...we could also make WebVector pickup a custom deleter to handle adopting a WTF::Vector type.  You'd need to modify swap() and assign(), and then likely store an extra function pointer to the deleter.

Not sure it's a good idea, but I *think* you bridge these APIs if you wanted...

-Albert

--

[Darin Fisher]

The original goal was to keep WebVector simple.  I'm not sure we want to complicate it too much.

If you want to minimize copies, then WebData / SharedBuffer seems like the way to go.  I notice that Adam advised against it, but the reference counting is what enables it to minimize copying.

Hmm...

-Darin

[William Chan (陈智昌)]

I of course know nothing about this area of the code, but I would just
like to mention that I dislike reference counting. That is all. Carry
on :)

> --

[Adam Barth]

I was hoping the changes to WebVector would be just two lines, but I
hadn't realized the issue with fastMalloc. Now WebData/SharedBuffer
seems like the better approach to me as well.

Adam

[Alec Flett]

So I'm going to go with the SharedBuffer version.. I welcome comments:

https://codereview.chromium.org/12217049/

https://bugs.webkit.org/show_bug.cgi?id=109092