649020 - Use JDCT_IFAST on mobile

Status: NEW

(Core :: Graphics: ImageLib, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Use JDCT_IFAST on mobile

Upstream android libjpeg used JDCT_IFAST so we probably should as well.

Comment 1

[Justin Lebar (not reading bugmail)]

Patch looks fine, but do you have any idea whether it's actually a perf improvement with our libjpeg?  I think I tested on x86 and didn't see any difference in speed between the IFAST, ISLOW, and the floating-point DCT.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #1)
> Patch looks fine, but do you have any idea whether it's actually a perf
> improvement with our libjpeg?  I think I tested on x86 and didn't see any
> difference in speed between the IFAST, ISLOW, and the floating-point DCT.

I haven't tested the perf difference, but I expect it would be more noticeable on ARM than on x86. I don't have an easy way of testing this right now. Siarhei, would you be interested in doing a quick measurement? Just building libjpeg-62 and comparing the speed of djpeg on a large jpeg should be a decent test for this.

Comment 3

[Siarhei Siamashka]

It depends on the availability of ARM NEON optimizations for different IDCT variants.

Comment 4

[Siarhei Siamashka]

FYI, some time ago we finally got permission to open source the old ARM NEON libjpeg6 optimizations from N900, and a quick port of the patches to libjpeg-turbo is available here: http://maemo.gitorious.org/meego-image-editor/libjpeg-turbo
Right now this set of patches only provides NEON version of IFAST, but ISLOW should be also possible with a bit more work.

Around the same time, linaro also started doing some work on ARM optimizations for libjpeg and basically I waited to see what happens there and whether they are going to work with upstream themselves: https://blueprints.launchpad.net/linaro-multimedia-wg/+spec/multimedia-linaro-optimize-jpeg-decoding

Comment 5

[Justin Lebar (not reading bugmail)]

(In reply to comment #3)
> It depends on the availability of ARM NEON optimizations for different IDCT
> variants.

My understanding is that we have no NEON code, for IDCT or anything else related to JPEGs.

Comment 6

[Justin Lebar (not reading bugmail)]

Comment on attachment 525084 [details] [diff] [review]
Use JDCT_IFAST on mobile

I'll r+ this, because it seems unlikely to make things slower.  But it would be really nice to know if it's any faster.

Comment 7

[Justin Lebar (not reading bugmail)]

Discussion about NEON IDCT routines is in bug 496298.

Comment 8

[Justin Lebar (not reading bugmail)]

Hm, this may have fallen through the cracks.  It's not in trunk, if it was ever checked in...

Comment 9

[Justin Lebar (not reading bugmail)]

Jeff, want to check this in?

Comment 10

[Justin Lebar (not reading bugmail)]

Consistent with https://bugs.webkit.org/show_bug.cgi?id=78323#c4 but contrary to comment 1 I see a small ~5% improvement with IDCT_FAST on desktop.  There are bigger fish to fry in our desktop image pipeline, fwiw.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

I think there were test failures last time I tried this and I never got around to disabling the tests on mobile.