Closed Bug 407600 Opened 12 years ago Closed 12 years ago

Compile SpiderMonkey with -Os on mac

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: sayrer, Assigned: crowderbt)

Details

(Keywords: perf)

Attachments

(2 files)

Right now it uses -O2 because that is the global setting. It should probably use MODULE_OPTIMIZE_FLAGS to indicate -Os. I have tried using -Os as the global setting for mac, but it was slightly slower overall on our pageload tests. Last time I tried this, I was getting mixed results at -Os, but now it is uniformly faster.

SunSpider results:

-Os
============================================
RESULTS (means and 95% confidence intervals)
--------------------------------------------
Total:                  6566.4ms +/- 1.4%
--------------------------------------------

  3d:                    896.5ms +/- 3.0%
    cube:                321.5ms +/- 2.5%
    morph:               307.7ms +/- 3.9%
    raytrace:            267.3ms +/- 3.5%

  access:               1177.8ms +/- 4.3%
    binary-trees:        104.9ms +/- 6.0%
    fannkuch:            510.4ms +/- 4.3%
    nbody:               327.9ms +/- 4.4%
    nsieve:              234.6ms +/- 6.2%

  bitops:                952.4ms +/- 1.1%
    3bit-bits-in-byte:    89.5ms +/- 0.9%
    bits-in-byte:        132.1ms +/- 1.8%
    bitwise-and:         491.3ms +/- 1.4%
    nsieve-bits:         239.5ms +/- 2.1%

  controlflow:            82.6ms +/- 1.1%
    recursive:            82.6ms +/- 1.1%

  crypto:                469.5ms +/- 4.0%
    aes:                 231.8ms +/- 5.3%
    md5:                 114.2ms +/- 1.2%
    sha1:                123.5ms +/- 5.0%

  date:                  557.6ms +/- 2.7%
    format-tofte:        369.5ms +/- 3.7%
    format-xparb:        188.1ms +/- 1.8%

  math:                  679.5ms +/- 3.4%
    cordic:              272.4ms +/- 2.7%
    partial-sums:        264.7ms +/- 1.9%
    spectral-norm:       142.4ms +/- 10.1%

  regexp:                574.1ms +/- 2.3%
    dna:                 574.1ms +/- 2.3%

  string:               1176.4ms +/- 2.9%
    base64:              231.6ms +/- 2.9%
    fasta:               286.9ms +/- 4.6%
    tagcloud:            230.5ms +/- 1.8%
    unpack-code:         306.9ms +/- 2.7%
    validate-input:      120.5ms +/- 6.4%

-O2
============================================
RESULTS (means and 95% confidence intervals)
--------------------------------------------
Total:                  7086.0ms +/- 4.1%
--------------------------------------------

  3d:                    972.3ms +/- 5.0%
    cube:                342.6ms +/- 4.2%
    morph:               338.5ms +/- 6.1%
    raytrace:            291.2ms +/- 7.8%

  access:               1250.0ms +/- 4.1%
    binary-trees:        119.4ms +/- 8.5%
    fannkuch:            510.6ms +/- 4.9%
    nbody:               376.5ms +/- 4.4%
    nsieve:              243.5ms +/- 6.0%

  bitops:               1013.2ms +/- 2.9%
    3bit-bits-in-byte:    94.0ms +/- 2.7%
    bits-in-byte:        139.1ms +/- 7.5%
    bitwise-and:         526.0ms +/- 3.0%
    nsieve-bits:         254.1ms +/- 3.4%

  controlflow:            83.2ms +/- 9.1%
    recursive:            83.2ms +/- 9.1%

  crypto:                493.9ms +/- 9.5%
    aes:                 250.8ms +/- 9.8%
    md5:                 115.7ms +/- 6.9%
    sha1:                127.4ms +/- 11.8%

  date:                  591.4ms +/- 8.2%
    format-tofte:        395.1ms +/- 8.5%
    format-xparb:        196.3ms +/- 8.8%

  math:                  740.4ms +/- 7.7%
    cordic:              295.0ms +/- 6.5%
    partial-sums:        295.2ms +/- 9.2%
    spectral-norm:       150.2ms +/- 8.2%

  regexp:                726.8ms +/- 5.4%
    dna:                 726.8ms +/- 5.4%

  string:               1214.8ms +/- 3.1%
    base64:              246.6ms +/- 3.8%
    fasta:               295.1ms +/- 2.7%
    tagcloud:            241.1ms +/- 5.1%
    unpack-code:         312.3ms +/- 3.5%
    validate-input:      119.7ms +/- 3.2%
Keywords: perf
Assignee: general → sayrer
Status: NEW → ASSIGNED
Comment on attachment 292465 [details] [diff] [review]
Always use the production settings when building Firefox. (changes Mac to -Os, like linux)

I have this building on the tryserver to check for bustage.
Attachment #292465 - Flags: superreview?(brendan)
Attachment #292465 - Flags: review?(brendan)
Comment on attachment 292465 [details] [diff] [review]
Always use the production settings when building Firefox. (changes Mac to -Os, like linux)

Would be best to get bsmedberg or luser or another build module owner or peer to r+ -- I can sr (I always build my optimized test js shells with -Os on Mac OS X; IIRC there's a bug about using -Os globally, not sure where that stands).

Optimistically pre-approving.

/be
Attachment #292465 - Flags: superreview?(brendan)
Attachment #292465 - Flags: superreview+
Attachment #292465 - Flags: review?(brendan)
Attachment #292465 - Flags: review?(benjamin)
Attachment #292465 - Flags: approval1.9+
(In reply to comment #3)
> (From update of attachment 292465 [details] [diff] [review])
> IIRC there's a bug about using -Os globally, not sure where that stands).

I tried it and it was ~2-5% slower on Tp/Ts/Tdhtml, but much smaller. I am going to try selectively building modules with -O2 on linux to see if there are a few more hotspots that would benefit from higher optimization (gfx and sqlite already do this).
Sadly this won't actually help our nightly builds, since we explicitly set optimize flags:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/macosx/mozconfig#15

We should probably change that to --enable-optimize and --enable-debugger-info-modules, although we still might be screwed if we want to do that on Linux, since we need to explicitly specify -gstabs+ there for breakpad.
Comment on attachment 292465 [details] [diff] [review]
Always use the production settings when building Firefox. (changes Mac to -Os, like linux)

Regardless of that, this patch is correct for what you want.  Maybe file a followup on the tinderbox mozconfigs?
Attachment #292465 - Flags: review?(benjamin) → review+
(In reply to comment #6)
> (From update of attachment 292465 [details] [diff] [review])
> Regardless of that, this patch is correct for what you want.  Maybe file a
> followup on the tinderbox mozconfigs?

Filed bug 407794.
This is the wrong way to solve this problem. Fixing bug 407794 will give us the correct defaults here. This build junk can be added later if necessary.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Actually, we should probably still address this for Makefile.ref
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(In reply to comment #9)
> Actually, we should probably still address this for Makefile.ref
> 

Fair enough. Should be -Os on gcc and -O1 for msvc.
Attached patch config.mkSplinter Review
Modifies the spidermonkey-local config.mk (the one used by Makefile.ref, shouldn't be used by Makefile.in -- someone please correct me if I am mistaken).
Assignee: sayrer → crowder
Attachment #292465 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #292477 - Flags: review?(ted.mielczarek)
Comment on attachment 292477 [details] [diff] [review]
config.mk

passing the buck
Attachment #292477 - Flags: review?(ted.mielczarek) → review?(benjamin)
Comment on attachment 292465 [details] [diff] [review]
Always use the production settings when building Firefox. (changes Mac to -Os, like linux)

I actually did land this as part of 407794, because we were unable to make -Os the default on mac.
Attachment #292465 - Attachment is obsolete: false
Attachment #292477 - Flags: review?(benjamin) → review+
config.mk: 3.19
Status: ASSIGNED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.