Comment on attachment 8919156 [details] Bug 1409265 - Update clang to 5.0 for linux and mac builds. https://reviewboard.mozilla.org/r/190072/#review195438 I guess we can't bump the minimum clang version until we get ASan builds on 5.0, right? ::: commit-message-c6a26:1 (Diff revision 1) > +Bug 1409265 - Use clang 5.0 for linux and mac builds. r?froydnj This is really "linux static analysis builds" and "all mac builds", correct? Just verifying my understanding here.

(In reply to Nathan Froyd [:froydnj] from comment #2) > Comment on attachment 8919156 [details] > Bug 1409265 - Use clang 5.0 for linux and mac builds. > > https://reviewboard.mozilla.org/r/190072/#review195438 > > I guess we can't bump the minimum clang version until we get ASan builds on > 5.0, right? Why should we? > ::: commit-message-c6a26:1 > (Diff revision 1) > > +Bug 1409265 - Use clang 5.0 for linux and mac builds. r?froydnj > > This is really "linux static analysis builds" and "all mac builds", correct? > Just verifying my understanding here. We use clang for normal builds now, for bindgen. That alters that too.

(In reply to Mike Hommey [:glandium] from comment #3) > (In reply to Nathan Froyd [:froydnj] from comment #2) > > Comment on attachment 8919156 [details] > > Bug 1409265 - Use clang 5.0 for linux and mac builds. > > > > https://reviewboard.mozilla.org/r/190072/#review195438 > > > > I guess we can't bump the minimum clang version until we get ASan builds on > > 5.0, right? > > Why should we? My thinking was that if we bumped the minimum clang version, (current) ASan builds on infra would start failing because they use 3.9. > > ::: commit-message-c6a26:1 > > (Diff revision 1) > > > +Bug 1409265 - Use clang 5.0 for linux and mac builds. r?froydnj > > > > This is really "linux static analysis builds" and "all mac builds", correct? > > Just verifying my understanding here. > > We use clang for normal builds now, for bindgen. That alters that too. Ah, right. OK.

(In reply to Nathan Froyd [:froydnj] from comment #5) > (In reply to Mike Hommey [:glandium] from comment #3) > > (In reply to Nathan Froyd [:froydnj] from comment #2) > > > Comment on attachment 8919156 [details] > > > Bug 1409265 - Use clang 5.0 for linux and mac builds. > > > > > > https://reviewboard.mozilla.org/r/190072/#review195438 > > > > > > I guess we can't bump the minimum clang version until we get ASan builds on > > > 5.0, right? > > > > Why should we? > > My thinking was that if we bumped the minimum clang version, (current) ASan > builds on infra would start failing because they use 3.9. Right, but why bump the minimum clang version?

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/64a4f7fc64e3 Update clang to 5.0 for linux and mac builds. r=froydnj

Backed out in https://hg.mozilla.org/integration/autoland/rev/08f1d46bc342 for unexpected assertion failures in Mac crashtests, https://treeherder.mozilla.org/logviewer.html#?job_id=137763477&repo=autoland and webaudio mochitests, https://treeherder.mozilla.org/logviewer.html#?job_id=137690275&repo=autoland

When this bug landed, as stated in comment 7, this regression was noticed: == Change summary for alert #10046 (as of October 17 2017 21:25 UTC) == Regressions: 2% installer size summary osx-cross opt 65,888,299.50 -> 66,902,163.25 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10046

The backout from comment 8 resolved that regression, as you can see here: == Change summary for alert #10059 (as of October 18 2017 02:59 UTC) == Improvements: 2% installer size summary osx-cross opt 66,926,784.17 -> 65,873,972.08 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10059

This other regression was noticed before the backout: == Change summary for alert #10049 (as of October 17 2017 21:25 UTC) == Regressions: 3% tsvgx summary osx-10-10 opt e10s 390.50 -> 403.05 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10049 After the backout: == Change summary for alert #10063 (as of October 17 2017 23:28 UTC) == Improvements: 4% tsvgx summary osx-10-10 opt e10s 404.33 -> 389.48 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10063

Well, it looks like this is not a no-brainer. If someone wants to give it a shot again, be my guest, but I won't actively work on this.

Also, before the backout, these improvements were noticed: == Change summary for alert #10047 (as of October 17 2017 21:25 UTC) == Improvements: 30% Strings PerfStripWhitespace osx-10-10 opt 134,489.50 -> 93,487.67 30% Strings PerfStripCRLF osx-10-10 opt 156,182.17 -> 109,669.67 7% Stylo Servo_DeclarationBlock_GetPropertyById_Bench osx-10-10 opt 202,991.33 -> 189,635.08 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10047 Results bellow confirm that the backout canceled them: == Change summary for alert #10061 (as of October 18 2017 02:59 UTC) == Regressions: 47% Strings PerfStripWhitespace osx-10-10 opt 88,549.83 -> 130,162.08 45% Strings PerfStripCRLF osx-10-10 opt 112,825.94 -> 163,085.33 12% Strings PerfStripCharsWhitespace osx-10-10 opt 365,659.12 -> 411,205.42 9% Stylo Servo_DeclarationBlock_GetPropertyById_Bench osx-10-10 opt 189,683.35 -> 207,088.27 2% Stylo Gecko_nsCSSParser_ParseSheet_Bench osx-10-10 opt 63,828.83 -> 65,415.58 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10061

The performance regressions from backout seem worse than those for the landing; I guess we should re-land this?

We can probably give it a second spin, especially considering the asan issues should all be fixed too. I'll do a new try run with a modified version of the patch to use clang-5 on asan too.

(In reply to Karl Tomlinson (:karlt) from comment #14) > The performance regressions from backout seem worse than those for the > landing; I guess we should re-land this? I suspect this is just fallout from the way we calculate percentages (i.e. a 25% increase can be countered by a 20% decrease). The raw values appear to be pretty similar before and after.

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/89093caa0f28 Update clang to 5.0 for linux and mac builds. r=froydnj

Thanks to push from comment 18, we ended up with some big perf improvements, according to our microbenchmarks: == Change summary for alert #11473 (as of Fri, 09 Feb 2018 01:30:07 GMT) == Improvements: 33% Strings PerfStripCRLF osx-10-10 opt 156,452.58 -> 105,145.42 27% Strings PerfStripWhitespace osx-10-10 opt 130,057.83 -> 94,965.92 22% Strings PerfStripCharsWhitespace osx-10-10 opt 403,472.42 -> 314,366.17 6% Stylo Servo_DeclarationBlock_GetPropertyById_Bench osx-10-10 opt 208,717.58 -> 196,424.92 3% Stylo Gecko_nsCSSParser_ParseSheet_Bench osx-10-10 opt 66,648.58 -> 64,788.08 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=11473