449130 - consider forcing -O2 for lcms on all platforms

Status: RESOLVED FIXED

(Core :: Graphics: Color Management, defect)

Description

[Bobby Holley (:bholley)]

right now mac builds lcms with -O2, win32 builds with -O1, and linux builds with -Os. I'm running perf numbers to see if there's a benefit to forcing -O2 on any platform.

Comment 2

[Bobby Holley (:bholley)]

This looks like a win. I just got the numbers back from the tryserver. Using the numbers I got on tuesday as a reference, we get 553->520 on linux, 607->597 on windows, and 358->358 on mac (this gives me some confidence in the numbers, since this patch should have zero effect on mac).

One thing to determine is how much effect this patch will have after precaching (presumably, most of the wins could be in code that became non-critical with that patch). I'm putting that on the tryserver now.

Comment 3

[Bobby Holley (:bholley)]

Attachment: proposed patch to force lcms to build -O2 on all platforms

The results with precaching are less exciting. We get 335->334 on mac, 490->488 on linux, and 498->495 on windows. To me this suggests that most of the perf win was in the linear interpolation, which is no longer on the critical path. Still, I think it's probably a good thing to have overall, though I could easily be persuaded otherwise. Flagging vlad for review.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 332407 [details] [diff] [review]
proposed patch to force lcms to build -O2 on all platforms

You just need it in the src dir, not in lcms/Makefile.in or lcms/include/Makefile.in ...  Also, put the MODULE_OPTIMIZE_FLAGS much later in the makefile, probably right before REQUIRES.

Comment 5

[Bobby Holley (:bholley)]

Attachment: updated patch

updated patch

Comment 6

[Bobby Holley (:bholley)]

perf numbers look good even with other optimizations. flagging checkin-needed.

Comment 7

[Bobby Holley (:bholley)]

pushed in fb356c4d33a1