firefox: add ltoSupport and enable it by default#99922
Conversation
|
cc @ajs124 |
andir
left a comment
There was a problem hiding this comment.
This actually looks good! I'll give it a test spin on our current firefox variants.
|
Each of the did build on my machine and appear to be working fine. I've not done any kind of benchmarks. The builds seem to take about 1 minute longer than usually on my 24 cores which is probably fine. |
|
I rebased this to solve the merge conflict that I caused in another PR. |
|
Using the Speedometer 2.0 on an Intel Core 2 Duo CPU T7250 @ 2.00GHz. Benchmark output
firefox-original: firefox-lto: firefox-bin: The benchmark seems to show a performance increase of ~9% when going from non-LTO to LTO. This is a considerably old machine so I am not sure how accurate the benchmark results are but I can give my opinion saying it feels smoother. |
LTO in general is broken on Darwin (see NixOS#19312).
|
This is looking good. Will try this one more time. |
worldofpeace
left a comment
There was a problem hiding this comment.
This LGTM 👍 Thanks a ton
This is also looking good here. @andir, did you do some more testing? |
|
@ofborg eval |
|
While this is quite an improvement, it will break reproducibility, right? (assuming it ever was reproducible) |
|
LTO should not have that much of an impact on that front. PGO might have an impact but we haven't added that yet. |
Ah sorry this is just about LTO, should be fine so far then |
Motivation for this change
Build Firefox with LTO support as upstream does. This change switches the compiler to clang and linker to lld.
The closure size increased by ~1% (586.6M -> 592.9M).
The resulting directory of interest did decrease in size (181M -> 179M).
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)