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.
On Tue, Dec 22, 2015 at 12:08 PM, Oded Gabbay <[email protected]> wrote: > On Tue, Dec 22, 2015 at 9:44 PM, Bill Spitzak <[email protected]> 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 <[email protected]> > wrote: > >> > >> On Sat, Dec 12, 2015 at 8:06 PM, <[email protected]> wrote: > >> > From: Bill Spitzak <[email protected]> > >> > > >> > 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 > >> > [email protected] > >> > 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 > > > > >
_______________________________________________ Pixman mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/pixman
