1579536 - Add transferables support to JS actor sendQuery

Status: RESOLVED WONTFIX

(Core :: DOM: Content Processes, enhancement, P2)

Description

[Mike Conley (:mconley) (:⚙️)]

sendQuery is a really nice way for the front-end team to offload the management of ping-ponging call and response messages to the JSWindowActor native layer.

We're able to use the sendQuery method to send Transferables from one actor to another - however, the receiver is not able to send a Transferable back. The data that the Promise resolves with seems to go through the normal structured clone stuff.

I'm not sure how to solve this - Promises are usually resolved with whatever data is passed as the first argument to the resolve method. Perhaps we could have a convention where if an Object is passed to resolve with a transferable (sic) property set, then we expect those things to be transferred to the the originator of the sendQuery. But I'm open to alternative suggestions.

What do you think, jdai?

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Until we have something like this, I think I'm going to have to switch away from using sendQuery in my fix for bug 1576565.

Comment 2

[John Dai[:jdai]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #0)

sendQuery is a really nice way for the front-end team to offload the management of ping-ponging call and response messages to the JSWindowActor native layer.

We're able to use the sendQuery method to send Transferables from one actor to another - however, the receiver is not able to send a Transferable back. The data that the Promise resolves with seems to go through the normal structured clone stuff.

I'm not sure how to solve this - Promises are usually resolved with whatever data is passed as the first argument to the resolve method. Perhaps we could have a convention where if an Object is passed to resolve with a transferable (sic) property set, then we expect those things to be transferred to the the originator of the sendQuery. But I'm open to alternative suggestions.

What do you think, jdai?

Do you have any specific use case need to send transferables from one actor to another? According to mdn, transferable can't use in cross process.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(In reply to John Dai[:jdai] from comment #2)

Do you have any specific use case need to send transferables from one actor to another? According to mdn, transferable can't use in cross process.

Huh. If that's true, what are the transfer's for in sendAsyncMessage and sendQuery?

What do they do? Have I greatly misunderstood their purpose? If so, we'll need to update the documentation.

Comment 4

[John Dai[:jdai]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)

(In reply to John Dai[:jdai] from comment #2)

Do you have any specific use case need to send transferable from one actor to another? According to mdn, transferable can't use in cross-process.

Huh. If that's true, what are the transfer's for in sendAsyncMessage and sendQuery?

The transferable for sendAsyncMessage was introduced by B2G, we don't use it in our codebase now. After discussed with Nika, we should remove the transfers attribute in JSWindowActor.

What do they do? Have I greatly misunderstood their purpose? If so, we'll need to update the documentation.

For cross-process case, the transferable objects' contents are copied into the serialized data (rather than just writing a pointer) [1].

Comment 5

[John Dai[:jdai]]

File bug 1580907 to remove transferables attribute for SendAsyncMessage and SendQuery in JSWindowActor.

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to John Dai[:jdai] from comment #5)

File bug 1580907 to remove transferables attribute for SendAsyncMessage and SendQuery in JSWindowActor.

After removing trasferables, what else do we need to do here? What's the next step?

Comment 7

[John Dai[:jdai]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #6)

(In reply to John Dai[:jdai] from comment #5)

File bug 1580907 to remove transferables attribute for SendAsyncMessage and SendQuery in JSWindowActor.

After removing trasferables, what else do we need to do here? What's the next step?

According to comment #4, the transferable attribute in sendAsyncMessage and sendQuery can't solve bug 1576565. I'm going to close it. Feel free to reopen it if you have different thought. :)

Comment 8

[Rob Wu [:robwu]]

This bug is linked from a source comment at sendQuery as Note that transfers are currently not supported by sendQuery. See Bug 1579536.

Bug 1580907 above dropped the transferable attribute from sendAsyncMessage and sendQuery.

Bug 1815339 restored the transferable attribute in sendAsyncMessage (starting with https://hg.mozilla.org/mozilla-central/rev/89cb8b7f9d10 at https://bugzilla.mozilla.org/show_bug.cgi?id=1815339#c18).

I'd also like to restore the transferable attribute in sendQuery to support bug 1910660. The purpose of using transferables there is to reduce the number of memory allocations, by allowing input buffers (StructuredCloneHolder instances specifically) to be owned instead of copied. StructuredCloneHolder can also offer an answer to the original question in this bug, of how to support transferables as responses. By default StructuredCloneHolder could make a (deep) copy, but we can also implement transferable semantics there to support sending transferables back (simply by letting the handler of the sendQuery call return a StructuredCloneHolder that wraps a transferable).

Mike - does that sound reasonable? Can we re-open this bug?

Comment 9

[Andrew McCreight [:mccr8]]

I mentioned this to Nika and she said she didn't think it would help much but I don't remember the specifics.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Sorry for the delayed response.

Mike - does that sound reasonable?

I personally think it does, from the perspective of someone who would consume this capability. I'll defer to our DOM team for the reasonable-ness of the underlying mechanism.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

I've left bug 1910660 comment 1 with some thoughts about the proposed approach in that bug.

I think I'd like to hold off on re-opening this bug, as I'm not sure that we'd want to use transferables to avoid the copy in the way being proposed in that original bug.

Comment 12

[Andrew McCreight [:mccr8]]

Asuth pointed me at some Google thing that supports transfers in kind of a general way, and I think it points at a way we could support those for replies: basically the idea would be you have like a WebIDL dict that is just a pair of two objects, and when you are getting ready to serialize a JS actor response, you first check for the special pair object. If you find it, you dig out the transfers object and use that for the serialization. (I'm looking over the translations code where it has to implement its own query reply in an awkward way so that it can send back a message port.)

Comment 13

[Andrew McCreight [:mccr8]]

I'll make this more specific to the current state: you can't send transferables via sendQuery, either in the initial message or the reply.