162095 - js\src should build with -O2

Status: RESOLVED DUPLICATE of bug 412210

(Core :: JavaScript Engine, defect)

Description

[Bernard Alleysson]

-O2 should give a perf boost

js.mak uses -O2 and http://www.mozilla.org/js/spidermonkey/apidoc/jsguide.html
gives js.mak to build:
"Under Windows NT the make file is jsmak"

So if -O2 is good for javascript embedding it should be the same for mozilla

Comment 1

[Bernard Alleysson]

Attachment: build js\src with -O2 by default (when --enable-optimize)

Comment 2

[Phil Schwartau]

Note Makefile.in and makefile.win were recently patched to fix
bug 151066: "Crash calling 'Variables' in jsparse.c"

We also have to keep js.mak, Makefile.ref in synch with those changes:
bug 160592: "Need to update js.mak, Makefile.ref, etc. for opt builds"

cc'ing Brendan, jrgm, rginda to review the current patch - should this
proposed change apply to any other makefile besides Makefile.in?

Comment 3

[John Morrison]

I haven't looked into how MODULE_OPTIMIZE_FLAGS is used, so I can't comment
on the patch (I should look soon, but...).

However, lines 327-339 of js/src/Makefile.in need to be removed. Otherwise, 
the -O2 will be replaced with the equivalent of -O1 for jsinterp.c. I had been
meaning to get them removed; we don't need them at all since lines 196-208 are
a complete fix for the optimized crash from bug 151066.

Comment 4

[Bernard Alleysson]

Adding keywords

"should this proposed change apply to any other makefile besides Makefile.in?"

js.mak doesn't need it (uses -O2 already)
if makefile.ref uses MSVC then the change should apply to makefile.ref
(I don't think that fdlibm\makefile.in needs -O2 because I don't think -O2 is
usefull to floating point operations)

"However, lines 327-339 of js/src/Makefile.in need to be removed"

I don't know. I'll update the patch so that -O2 is not lost for jsinterp.c

Comment 5

[Bernard Alleysson]

Attachment: patch v2

change the test to exclude GNU_CC (this is msvc specific)
replace -O2 by its equivalent

Comment 6

[Bernard Alleysson]

I've got some numbers:

run "xpcshell -f all_bench.js (from js\benchmarks)"

-O2: 37264 37253 37333 (ms) avg : 37283 (js3250.dll is 376 Ko)
-O1: 40278 40269 40317 (ms) avg : 40288 (js3250.dll is 320 Ko)

so the perf gain is 7.5 % and the cost is 56 Ko additional code (17.5 %)

I'll attach the test outputs for a particular run in case you want to see the
details

Comment 7

[Bernard Alleysson]

Attachment: all_bench.js output with -O1

Comment 8

[Bernard Alleysson]

Attachment: all_bench.js output with -O2

Comment 9

[Stephen Walker]

so, is anything going to happen with the latest patch? 

brendan, what's your opinion on the bloat/perf tradeoff?

Comment 10

[Markus Hübner]

Wouldn't this be a good thing to consider for 1.4a ?

Comment 11

[Brendan Eich [:brendan]]

Why isn't -O2 used for the whole Windows Firefox (or other app) build?  That's
what Makefile.in partakes of.  Is this bug really about Makefile.in, or has it
morphed?  It's quite old, and comment 0 talks about js.mak (part of the old
standalone "jsref" build).

/be

Comment 12

[Bernard Alleysson]

(In reply to comment #11)
> Why isn't -O2 used for the whole Windows Firefox (or other app) build?

This bug was inspired by this comment:
https://bugzilla.mozilla.org/show_bug.cgi?id=136999#c13

The idea was to identify a subset of modules to compile with -O2, for instance
one would make a firefox build with html, css, image decoders, javascript,
xpcom, ... compiled with -O2 and keep SVG, mathml, extensions, plugins, setup,
... at -O1, to optimize size vs performance (I would call it -O1.5 build)

>  Is this bug really about Makefile.in, or has it morphed?
Yes, it's about Makefile.in 
It's about evaluating the size/performance tradeoff of compiling js\src with -O2

Comment 13

[Brian Crowder]

I'd like to volunteer to drive this to completion and hopefully do the same for Makefile.ref, if we can identify whatever shortcomings are preventing it from getting landed.  The benchmark results are relatively compelling, even taken with as many grains of salt as benchmarks deserve.