Last Comment Bug 675568 - SIMD of libjpeg turbo is disabled on Windows
: SIMD of libjpeg turbo is disabled on Windows
: regression
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All Windows Vista
: -- normal (vote)
: mozilla8
Assigned To: Makoto Kato [:m_kato]
Depends on:
Blocks: 652399
  Show dependency treegraph
Reported: 2011-07-31 22:41 PDT by Makoto Kato [:m_kato]
Modified: 2011-09-22 16:40 PDT (History)
11 users (show)
khuey: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix (1.12 KB, patch)
2011-07-31 22:45 PDT, Makoto Kato [:m_kato]
khuey: review-
Details | Diff | Splinter Review
fix v2 (2.65 KB, patch)
2011-08-05 02:02 PDT, Makoto Kato [:m_kato]
khuey: review+
Details | Diff | Splinter Review
for aurora (2.00 KB, patch)
2011-08-07 21:47 PDT, Makoto Kato [:m_kato]
jpr: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Makoto Kato [:m_kato] 2011-07-31 22:41:02 PDT
SKIP_LIBRARY_CHECKS is "yes" on Windows, so SIMD of libjpeg turbo is turned off.  We should turn on.
Comment 1 Makoto Kato [:m_kato] 2011-07-31 22:45:40 PDT
Created attachment 549720 [details] [diff] [review]
Comment 2 Justin Lebar (not reading bugmail) 2011-08-01 07:54:58 PDT
Do we have any idea when this changed?
Comment 3 Kyle Huey [:khuey] ( 2011-08-01 07:55:41 PDT
(In reply to comment #2)
> Do we have any idea when this changed?

According to Bug 652399, for Gecko 6.
Comment 4 Justin Lebar (not reading bugmail) 2011-08-01 07:58:29 PDT
Setting tracking-? for FF6-8.  This is a major (factor of 2?) jpeg decoding regression.  This matters particularly much now that we aggressively discard images in background tabs.

Risk of taking this even on beta is low, because we had libjpeg-turbo enabled in FF4 and FF5.
Comment 5 Kyle Huey [:khuey] ( 2011-08-01 10:15:18 PDT
Comment on attachment 549720 [details] [diff] [review]

We should fix this by moving the added check down to the libjpeg-turbo section.

We should also make it so that ./configure --with-system-jpeg --enable-libjpeg-turbo errors out.
Comment 6 Justin Lebar (not reading bugmail) 2011-08-01 11:27:43 PDT
I backed out bug 652399 on beta:

I didn't back out on Aurora, so we'll want to take this fix on Aurora.
Comment 7 Justin Lebar (not reading bugmail) 2011-08-01 16:40:39 PDT
The push from comment 6 was to the relbranch, not the default branch. Properly backed out on beta:
Comment 8 Makoto Kato [:m_kato] 2011-08-05 02:02:57 PDT
Created attachment 550975 [details] [diff] [review]
fix v2
Comment 9 Kyle Huey [:khuey] ( 2011-08-07 19:09:15 PDT
Comment on attachment 550975 [details] [diff] [review]
fix v2

>@@ -6317,16 +6316,20 @@ if test -n "$MOZ_LIBJPEG_TURBO"; then
>+    if test "$SYSTEM_JPEG" = 1; then
>+        AC_MSG_ERROR([cannot use --with-system-jpeg with --enable-libjpeg-turbo.])
>+    fi

I think this should be right after the MOZ_ARG_DISABLE_BOOL for libjpeg-turbo.

r=me with that.
Comment 10 Makoto Kato [:m_kato] 2011-08-07 20:21:09 PDT
landed with khuey's comment
Comment 11 Makoto Kato [:m_kato] 2011-08-07 21:47:44 PDT
Created attachment 551389 [details] [diff] [review]
for aurora
Comment 12 Kyle Huey [:khuey] ( 2011-08-08 05:50:09 PDT
Comment 14 Simona B [:simonab] 2011-08-19 04:43:54 PDT
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0

Can anyone please help me with a test case or with STR / guidelines that I can use to verify this fix?

Comment 15 Justin Lebar (not reading bugmail) 2011-08-22 10:12:08 PDT
Find a very large JPEG image (not a solid color).  Load it in a tab.  In a new tab, load about:memory and click "minimize memory usage" -- this should cause the image to be discarded.  Now switch back to the tab with the image.  Time with a stopwatch how long it takes before the image is displayed onscreen.  It should be significantly faster after this change than before.

Alternatively, you could profile a release build under a workload which spends most of its time decoding JPEGs, and examine the stack traces to see whether it's spending time in C or ASM routines.

You could also run a build in a debugger and see whether it's spending time in an ASM routine, executing SSE2 code.
Comment 16 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 16:40:21 PDT
qa- as no QA fix verification needed

Note You need to log in before you can comment on or make changes to this bug.