[Pixman] [PATCH 07/15] pixman-filter: Speed up the BOX+BOX filter

[Oded Gabbay]

On Tue, Dec 22, 2015 at 10:53 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> I think the improvement is obvious if you check how much code is run to do
> the Simpson's integration. BOX.BOX will literally be the filter used almost
> always once this is finally fixed.
>

I understand the logic behind this, and I can agree with it, but the
standard in pixman, AFAIK, is to show actual performance improvements
using benchmarks/real-world cases.

I'm not against this patch, and I think it will be eventually merged.
I'm just saying I think we need to defer it until:

1. The changes you talk about them are actually merged into pixman & cairo
2. We can show the (hopefully major) improvement using cairo benchmarking.

Thanks,

       Oded

>
> On Tue, Dec 22, 2015 at 12:08 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>>
>> On Tue, Dec 22, 2015 at 9:44 PM, Bill Spitzak <spitzak at gmail.com> wrote:
>> > Any test using Cairo is not using this code for scaling down (since it
>> > uses
>> > it's own filter generator, or older Cairo which only used bilinear) so I
>> > am
>> > not sure if this case is being hit. If GOOD in the future produces
>> > BOX.BOX
>> > then this will be hit a lot more often.
>> >
>> OK, got it. Thanks.
>>
>> So do you have some other case where you can demonstrate the
>> effectiveness of this optimization ?
>>
>> If not, then I think we need to defer this patch until such a case
>> arises, e.g. what you wrote about GOOD producing BOX.BOX in the
>> future.
>>
>>        Oded
>>
>> >
>> >
>> > On Tue, Dec 22, 2015 at 1:33 AM, Oded Gabbay <oded.gabbay at gmail.com>
>> > wrote:
>> >>
>> >> On Sat, Dec 12, 2015 at 8:06 PM,  <spitzak at gmail.com> wrote:
>> >> > From: Bill Spitzak <spitzak at gmail.com>
>> >> >
>> >> > This is easy as the caller already intersected the two boxes, so
>> >> > the width is the integral.
>> >> > ---
>> >> >  pixman/pixman-filter.c | 5 +++++
>> >> >  1 file changed, 5 insertions(+)
>> >> >
>> >> > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> >> > index 4aafa51..782f73d 100644
>> >> > --- a/pixman/pixman-filter.c
>> >> > +++ b/pixman/pixman-filter.c
>> >> > @@ -182,6 +182,11 @@ integral (pixman_kernel_t reconstruct, double
>> >> > x1,
>> >> >         assert (width == 0.0);
>> >> >         return filters[sample].func (x2 / scale);
>> >> >      }
>> >> > +    else if (reconstruct == PIXMAN_KERNEL_BOX && sample ==
>> >> > PIXMAN_KERNEL_BOX)
>> >> > +    {
>> >> > +       assert (width <= 1.0);
>> >> > +       return width;
>> >> > +    }
>> >> >      else if (sample == PIXMAN_KERNEL_IMPULSE)
>> >> >      {
>> >> >         assert (width == 0.0);
>> >> > --
>> >> > 1.9.1
>> >> >
>> >> > _______________________________________________
>> >> > Pixman mailing list
>> >> > Pixman at lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/pixman
>> >>
>> >>
>> >> As Soren said in his original email, this specialized case is
>> >> justified if we can demonstrate an improvement in a real-world
>> >> use-case, while making sure there aren't any other regressions.
>> >>
>> >> Generally speaking, when adding specific conditions to optimize code
>> >> we need to see evidence that indeed the new code is faster in *most*
>> >> cases. This is because even if the added conditions improve
>> >> performance of a specific use-case, it might actually degrade
>> >> performance on most other cases as they now need to do additional
>> >> comparisons in every pass of this code.
>> >>
>> >> Therefore, I think we need to see some real numbers to accept this
>> >> patch.
>> >>
>> >> fyi, I did a cairo benchmark run (on the trimmed benchmarks), and it
>> >> was practically unchanged. When I checked the results with
>> >> "--min-change 1%", I got:
>> >>
>> >> Speedups
>> >> ========
>> >> image  t-firefox-canvas-swscroll  691.34 (701.92 1.30%) -> 678.93
>> >> (693.67 1.25%):  1.02x speedup
>> >>
>> >> image         t-firefox-fishtank  1611.65 (1640.22 1.23%) -> 1591.66
>> >> (1653.68 1.91%):  1.01x speedup
>> >>
>> >> Slowdowns
>> >> =========
>> >> image     t-gnome-system-monitor  886.06 (893.33 1.20%) -> 895.76
>> >> (903.89 1.32%):  1.01x slowdown
>> >>
>> >> image         t-firefox-fishbowl  3242.06 (3280.91 0.55%) -> 3284.20
>> >> (3285.17 0.08%):  1.01x slowdown
>> >>
>> >> image                t-evolution  335.63 (337.11 0.28%) -> 340.48
>> >> (352.97 1.62%):  1.01x slowdown
>> >>
>> >> image        t-xfce4-terminal-a1  554.95 (572.35 1.59%) -> 563.67
>> >> (582.91 1.62%):  1.02x slowdown
>> >>
>> >> image       t-gnome-terminal-vim  354.85 (358.67 0.67%) -> 364.49
>> >> (366.12 0.33%):  1.03x slowdown
>> >>
>> >>       Oded
>> >
>> >
>
>