Closed Bug 1572216 Opened 5 years ago Closed 5 years ago

various regressions from attempting to turn off xLTO on non-shippable builds

Categories

(Firefox Build System :: General, defect)

All
Windows
defect
Not set
normal

Tracking

(firefox-esr60 unaffected, firefox-esr68 unaffected, firefox68 unaffected, firefox69 unaffected, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox68 --- unaffected
firefox69 --- unaffected
firefox70 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

No description provided.

Some recent changes to how we set cross-language LTO for Windows
resulted in compilation-time decreases and small performance regressions
on a few benchmarks. The changes intended to remove explicit enablement
of cross-language LTO for all builds, but rely on shippable builds being
built with PGO and moz.configure's clever defaulting of cross-language
LTO for PGO'd builds on Windows, which would then enable cross-language
LTO for only shippable builds.

Obviously something went wrong with those changes.

The problem is where we set the default for cross-language LTO. We set
it after the value for --enable-lto has been determined, and that
value defaults to off (--disable-lto). Since moz.configure is very
thorough in passing configure options down into JS, it dutifully looked
at what the default value of --enable-lto was supposed to be, and
passed --disable-lto to JS's configure. Our attempted defaulting of
--enable-lto to cross only did this in the Windows PGO case when the
value of --enable-lto was defaulted--which in the case of JS wasn't
true: the value was coming from the explicitly-passed command-line
option of --disable-lto. So JS didn't enable any kind of LTO, with
attendant consequences.

This problem didn't happen before the aforementioned change because we
were explicitly specifying that --enable-lto=cross should be passed in
the mozconfig, which ensured that the correct setting was passed into JS.

The fix is straightforward: we should move the option defaulting to
someplace where moz.configure can see it, thereby passing the correct
setting to JS in all cases.

Attachment #9083800 - Attachment description: Bug 1572216 - move computation of LTO default into a proper default; r=#build → Bug 1572216 - move LTO defaulting into mozconfig.win-common; r=#build
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c2141e083838 move LTO defaulting into mozconfig.win-common; r=glandium
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

== Change summary for alert #22443 (as of Wed, 14 Aug 2019 05:06:01 GMT) ==

Improvements:

17% raptor-tp6-youtube-firefox loadtime windows7-32-shippable opt 969.40 -> 800.92
17% raptor-tp6-youtube-firefox loadtime windows10-64-shippable opt 986.65 -> 822.17
15% raptor-tp6-youtube-firefox loadtime windows7-32-shippable opt 970.04 -> 821.33
9% raptor-tp6-twitch-firefox fcp windows7-32-shippable opt 47.90 -> 43.50
6% raptor-tp6-youtube-firefox windows10-64-shippable-qr opt 571.28 -> 534.30
5% raptor-ares6-firefox windows10-64-shippable opt 60.08 -> 56.79
5% raptor-ares6-firefox windows10-64-shippable opt 59.82 -> 56.90
4% raptor-ares6-firefox windows10-64-shippable-qr opt 59.75 -> 57.17
4% raptor-speedometer-firefox windows10-64-shippable-qr opt 83.06 -> 86.28
3% raptor-tp6-youtube-firefox-cold fcp windows10-64-shippable-qr opt 694.96 -> 670.83
3% raptor-tp6-sheets-firefox loadtime windows10-64-shippable opt 925.08 -> 895.50
3% raptor-tp6-sheets-firefox loadtime windows10-64-shippable-qr opt 927.06 -> 898.04
3% raptor-tp6-tumblr-firefox loadtime windows7-32-shippable opt 627.02 -> 608.42
3% raptor-tp6-tumblr-firefox loadtime windows10-64-shippable-qr opt 635.12 -> 617.17
3% raptor-tp6-yahoo-news-firefox fcp windows7-32-shippable opt 335.23 -> 326.33
2% raptor-tp6-yahoo-news-firefox windows7-32-shippable opt 373.95 -> 365.82

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22443

Regressions: 1579596
No longer regressions: 1579596
Regressions: 1579929
Regressions: 1579596
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: