Re: Having more control over when an IPDL message is compressed
On 3/26/14 10:13 PM, Botond Ballo wrote: The name "compression" is unfortunate here. It took me some time to understand what the real issue is. Agreed. (Any ideas for a better word? For me 'supersede' comes to mind, as in a newer message supersedes an older one so the older one can be dropped, but I don't think that would be very clear either.) "coalesced"? -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Having more control over when an IPDL message is compressed
> The name "compression" is unfortunate here. It took me some time to > understand what the real issue is. Agreed. (Any ideas for a better word? For me 'supersede' comes to mind, as in a newer message supersedes an older one so the older one can be dropped, but I don't think that would be very clear either.) > That said, it seems like the "clean" architectural solution here is to > create a new actor for every combination of parameters that you do not > want to "compress" messages for, and leave as function parameters > every parameter that you do want to compress messages for. That may > not scale well, but in this case I think it would just mean creating > an actor for every (sub?)frame. Have we tried that? Is the > performance cost too high? Interesting idea. My intuition tells me that this would come with a lot of overhead, as right now we have a single TabChild per browser tab (persistent across loading new pages in the tab), while scrollable frames are specific to each page (and sometimes come and go while navigating a single page). It also seems like having to coordinate the creation and destruction of these actors would add complexity, but I'm not very familiar with IPDL so I could be wrong. My immediate interest in this has diminished somewhat as my other use case for conditional compression (PBrowser::RealTouchMoveEvent) has gone away due to a change in approach for bug 976605, and I don't think the overhead of not compressing multiple UpdateFrame messages for the same frame is enough to justify doing it just for that. I'll keep my eyes open for other uses cases, though, and keep your suggestion of creating more actors in mind. Cheers, Botond ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Having more control over when an IPDL message is compressed
On Tue, Mar 25, 2014 at 6:18 AM, Botond Ballo wrote:
> Hello dev-platform,
>
> I recently fixed an APZ bug [1] that was caused by an IPDL message,
> PBrowser::UpdateFrame, being compressed when it shouldn't have been.
>
> I think the compression was correct back when we didn't have subframe
> scrolling, and so all messages pertained to the same frame, in which
> case it's appropriate to only act on the most recent message. With
> subframe scrolling, subsequent messages could apply to different
> frames, and dropping all but the most recent message could lead to a
> frame missing an important update.
>
> I fixed the bug by getting rid of the compression, but it occurred to
> me that if we had a mechanism for specifying that two messages
> applying to the same frame could be suppressed, but two messages
> applying to different frames could not, we'd probably want to use it.
>
> Is there any interest in adding such a mechanism to IPDL?
>
> Here's a straw man proposal for how it could work. Obviously, the
> syntax is not the interesting part and I'm happy to change it to
> whatever other syntax people might like:
>
> In addition to having a 'compress' attribute, have a 'compress_if'
> attribute that takes a function name as argument. Such a function
> would be expected to be provided in C++ code, to have 2*n arguments,
> where 'n' is the argument to the message, and to return 'bool'
> indicating whether or not two messages with those arguments should
> be suppressed.
>
> For example:
>
> RetType myMessage(Foo foo, Bar bar) compress_if(CompressMyMessage);
>
> And then one would provide:
>
> bool CompressMyMessage(const Foo& foo1, const Bar& bar1,
> const Foo& foo2, const Bar& bar2)
> {
> // whatever
> }
>
> which would decide whether or not myMessage(foo1, bar1) and
> myMessage(foo2, bar2) should be compressed.
>
> For example, for UpdateFrame we could do:
>
> UpdateFrame(FrameMetrics frame) compress_if(CompressUpdateFrame);
>
> and provide:
>
> bool CompressUpdateFrame(const FrameMetrics& frame1,
>const FrameMetrics& frame2)
> {
> // Two UpdateFrame messages can be compressed if they
> // apply to the same frame.
> return frame1.mScrollId == frame2.mScrollId;
> }
>
> I can think of at least one other use case in code that I'm
> currently working on: for bug 976605 [2], I'd find it
> convenient to conditionally compress PBrowser::RealTouchMoveEvent,
> to make sure that certain touch-move events (those with a
> particular flag set which is important for the child side to
> know about) are not compressed away.
>
> Presumably there will be other use cases in other protocols as well.
>
> Any thoughts about this / interest in this?
>
> Cheers,
> Botond
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=984673
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=976605
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
The name "compression" is unfortunate here. It took me some time to
understand what the real issue is.
That said, it seems like the "clean" architectural solution here is to
create a new actor for every combination of parameters that you do not
want to "compress" messages for, and leave as function parameters
every parameter that you do want to compress messages for. That may
not scale well, but in this case I think it would just mean creating
an actor for every (sub?)frame. Have we tried that? Is the
performance cost too high?
- Kyle
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Having more control over when an IPDL message is compressed
On Mon, Mar 24, 2014 at 03:18:19PM -0700, Botond Ballo wrote:
> Hello dev-platform,
>
> I recently fixed an APZ bug [1] that was caused by an IPDL message,
> PBrowser::UpdateFrame, being compressed when it shouldn't have been.
>
> I think the compression was correct back when we didn't have subframe
> scrolling, and so all messages pertained to the same frame, in which
> case it's appropriate to only act on the most recent message. With
> subframe scrolling, subsequent messages could apply to different
> frames, and dropping all but the most recent message could lead to a
> frame missing an important update.
>
> I fixed the bug by getting rid of the compression, but it occurred to
> me that if we had a mechanism for specifying that two messages
> applying to the same frame could be suppressed, but two messages
> applying to different frames could not, we'd probably want to use it.
>
> Is there any interest in adding such a mechanism to IPDL?
seems reasonable enough.
> Here's a straw man proposal for how it could work. Obviously, the
> syntax is not the interesting part and I'm happy to change it to
> whatever other syntax people might like:
>
> In addition to having a 'compress' attribute, have a 'compress_if'
> attribute that takes a function name as argument. Such a function
> would be expected to be provided in C++ code, to have 2*n arguments,
> where 'n' is the argument to the message, and to return 'bool'
> indicating whether or not two messages with those arguments should
> be suppressed.
>
> For example:
>
> RetType myMessage(Foo foo, Bar bar) compress_if(CompressMyMessage);
>
> And then one would provide:
>
> bool CompressMyMessage(const Foo& foo1, const Bar& bar1,
> const Foo& foo2, const Bar& bar2)
> {
> // whatever
> }
There's a part of me that wants to do this with a well known name for a
static method on the actor (though I guess that's tricky since the code
gen doesn't currently know the static type of the actor) but then you
could write
bool
FooChild::CoalesceMyMessage(const Bar& aM1, const bar& aM2)
{
return something or other;
}
Then we could use this same mechanism to implement always compressing,
but maybe that's too magical...
Trev
>
> which would decide whether or not myMessage(foo1, bar1) and
> myMessage(foo2, bar2) should be compressed.
>
> For example, for UpdateFrame we could do:
>
> UpdateFrame(FrameMetrics frame) compress_if(CompressUpdateFrame);
>
> and provide:
>
> bool CompressUpdateFrame(const FrameMetrics& frame1,
>const FrameMetrics& frame2)
> {
> // Two UpdateFrame messages can be compressed if they
> // apply to the same frame.
> return frame1.mScrollId == frame2.mScrollId;
> }
>
> I can think of at least one other use case in code that I'm
> currently working on: for bug 976605 [2], I'd find it
> convenient to conditionally compress PBrowser::RealTouchMoveEvent,
> to make sure that certain touch-move events (those with a
> particular flag set which is important for the child side to
> know about) are not compressed away.
>
> Presumably there will be other use cases in other protocols as well.
>
> Any thoughts about this / interest in this?
>
> Cheers,
> Botond
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=984673
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=976605
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
