[READY] Optionally enable Link Time Optimisation (LTO) #1540
Conversation
|
Benchmark results. The No-LTO and LTO values are taken from a median of 5 runs to eliminate any anomalies Benchmarks
|
There was a problem hiding this comment.
Wow... Reviewable doesn't know how to render tables...
Anyway, LTO seems to be better with a small dataset and progressively get worse as the use cases get worse. The PythonSupportWhatever benchmarks for really heavy workloads didn't improve and those are our weakest points.
Reviewed 2 of 2 files at r1.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @inglor)
cpp/CMakeLists.txt, line 231 at r1 (raw file):
# diagnostics. include(CheckIPOSupported) check_ipo_supported( RESULT LTOAvailable )
We can just assume this is available, because we only support clang, gcc and msvc.
cpp/ycm/CMakeLists.txt, line 446 at r1 (raw file):
############################################################################### if ( LTOAvailable )
Do we want this in debug builds?
There was a problem hiding this comment.
Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)
cpp/CMakeLists.txt, line 231 at r1 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
We can just assume this is available, because we only support clang, gcc and msvc.
Done.
cpp/ycm/CMakeLists.txt, line 446 at r1 (raw file):
Previously, bstaletic (Boris Staletic) wrote…
Do we want this in debug builds?
Done.
inglor
changed the title
inglor
force-pushed
the
enable_link_optimization
branch
from
a7d607d
to
9583a2b
Compare
LTO provides smaller, faster executables/DSOs, and improves GCC diagnostics. https://llvm.org/docs/LinkTimeOptimization.html https://gcc.gnu.org/wiki/LinkTimeOptimization https://cmake.org/cmake/help/latest/module/CheckIPOSupported.html
inglor
force-pushed
the
enable_link_optimization
branch
from
9583a2b
to
7d36556
Compare
Codecov Report
@@ Coverage Diff @@
## master #1540 +/- ##
==========================================
+ Coverage 96.32% 96.37% +0.05%
==========================================
Files 90 90
Lines 7839 7839
Branches 164 164
==========================================
+ Hits 7551 7555 +4
+ Misses 235 231 -4
Partials 53 53 |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
There was a problem hiding this comment.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)
There was a problem hiding this comment.
Reviewable status:
complete! 2 of 2 LGTMs obtained (waiting on @bstaletic)
There was a problem hiding this comment.
@puremourning My only concern with this are the benchmarks. Me and @inglor have both measured that LTO makes things from ~3% faster to ~3% slower. CI agrees, except on Winblows where it... let me check... all benchmarks turned out to be 10% slower with LTO. I would be fine with 3%, but 10%?
Now, that is CI, which is running some unknown CPU with an unknown load. Not exactly a perfect measuring device.
Your thoughts?
Dismissed @mark2185 from a discussion.
Reviewable status:
|
Thanks for sending a PR! |
There was a problem hiding this comment.
Reviewable status:
a discussion (no related file):
Cancel.
There was a problem hiding this comment.
Your thoughts?
If LTO does not produce a measurable, reliable improvement, then there is no real point in the change. My intuition is that 3% of the "fast part" is not going to move the needle when the "slow part" is added back (*he said, without any measurements to prove this), even if the 3% is in our favour.
We certainly don't want a 10% regression, that's for sure.
Reviewable status:
There was a problem hiding this comment.
I will close this and revisit once we get newer compiler versions on CI.
Reviewable status:
a discussion (no related file):
Previously, puremourning (Ben Jackson) wrote…
Cancel.
Done.
LTO provides smaller, faster executables / DSOs, and improves GCC diagnostics.
https://llvm.org/docs/LinkTimeOptimization.html
https://gcc.gnu.org/wiki/LinkTimeOptimization
https://cmake.org/cmake/help/latest/module/CheckIPOSupported.html
This change is