Typemapping of mojo::Arrays?

48 views
Skip to first unread message

Fady Samuel

unread,
Jun 14, 2016, 7:38:23 AM6/14/16
to chromium-mojo
There are a couple of cases that I know of where we'd like to pass around arrays through interface methods:


Currently the naive solution is to use type converters to an from mojo::Arrays which results in extra copies. A slightly better solution is to wrap the array in a struct and use StructTraits to map to an std::vector. This works, although maybe a little bit clumsy.

Does typemapping of mojo::Arrays make sense? Thanks,

Fady

Ken Rockot

unread,
Jun 14, 2016, 10:05:37 AM6/14/16
to Fady Samuel, chromium-mojo

Well, for probably the most common case (std::vector) we'll eventually just change the bindings to use this by default. I think that's still a goal but Yuzhu can confirm.

For other cases wrapping in a struct doesn't seem too clumsy to me and the cost seems negligible.

>
> Does typemapping of mojo::Arrays make sense? Thanks

Kind of, but I don't think we should do it. Lots of complexity for relatively little payoff.

>
> Fady
>
> --
> You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
> To post to this group, send email to chromi...@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAMZzjT%3DkK-9X7rv15%3DH6O07kkQV2AAcFUvmxwuf7KkPsV1-U_Q%40mail.gmail.com.

Yuzhu Shen

unread,
Jun 14, 2016, 11:58:26 AM6/14/16
to Ken Rockot, Fady Samuel, chromium-mojo
On Tue, Jun 14, 2016 at 7:05 AM, Ken Rockot <roc...@chromium.org> wrote:


On Jun 14, 2016 4:38 AM, "Fady Samuel" <fsa...@chromium.org> wrote:
>
> There are a couple of cases that I know of where we'd like to pass around arrays through interface methods:
>
> 1. https://codereview.chromium.org/1992443002/patch/200001/210008
> 2. https://cs.chromium.org/chromium/src/components/mus/public/interfaces/compositor_frame.mojom?q=compositor_frame.mojom&sq=package:chromium&l=33
>
> Currently the naive solution is to use type converters to an from mojo::Arrays which results in extra copies. A slightly better solution is to wrap the array in a struct and use StructTraits to map to an std::vector. This works, although maybe a little bit clumsy.

Well, for probably the most common case (std::vector) we'll eventually just change the bindings to use this by default. I think that's still a goal but Yuzhu can confirm.

If you just need std::vector, the easiest way is to move std::vector into/out of mojo::Array, which is very cheap to do:

mojo::Array<Foo> mojo_array(std::move(std_vector));
std::vector<Foo> std_vector = mojo_array.PassStorage();

Using std::vector<> by default requires:
- mapping non-nullable and nullable array to different types (std::vector, base::Optional<std::vector>)
- how to pass std::vector<Foo> as parameter (Foo may or may not be move-only)
  -- always pass by value, that may cause accidental copy.
  -- always pass by rvalue, that is against our style guide.
  -- depends on whether Foo is move-only, choose pass-by-value and pass-by-const-ref. The knowledge whether Foo is move-only has to be available at code generation time. That means if Foo is a custom type, we need to annotate it in .typemap file. (Probably we can rename the "pass-by-value" annotation and change its definition slightly.)

This is doable, but I am not super sure that the benefit outweighs the cost (code complexity as well as users' learning cost). WDYT?
 

For other cases wrapping in a struct doesn't seem too clumsy to me and the cost seems negligible.

I also think it is okay to do this. The cost is 8 extra bytes on the wire, which seems okay.
 

>
> Does typemapping of mojo::Arrays make sense? Thanks

Kind of, but I don't think we should do it. Lots of complexity for relatively little payoff.

You probably don't mean typemapping all mojo::Arrays of chromium ( or all arrays in blink) to a custom type, right? Then type aliasing is also required to map some mojo::Arrays but not all. 
Agreed with Ken that it probably doesn't worth the effort.

(That being said, I do want to support blanketly typemapping mojo::Array to WTFArray for blink, so we can remove WTF knowledge from mojo/public.)


 

>
> Fady
>
> --
> You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
> To post to this group, send email to chromi...@chromium.org.
> To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-mojo/CAMZzjT%3DkK-9X7rv15%3DH6O07kkQV2AAcFUvmxwuf7KkPsV1-U_Q%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "chromium-mojo" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-moj...@chromium.org.
To post to this group, send email to chromi...@chromium.org.
Reply all
Reply to author
Forward
0 new messages