Bug 235979

Summary: [CMake] Use ld.lld by default for development builds where available
Product: WebKit Reporter: Adrian Perez <aperez>
Component: CMakeAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, aperez, bugs-noreply, cgarcia, darin, don.olmstead, ews-watchlist, gyuyoung.kim, mcatanzaro, mrobinson, pgriffis, ross.kirsling, ryuan.choi, sergio
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=236362
https://bugs.webkit.org/show_bug.cgi?id=236506
Bug Depends on: 235476, 235975    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch v2 none

Description Adrian Perez 2022-02-01 15:03:33 PST
One of the reasons to use ld.gold for development builds before trying
to not detect it (as per bug #235975) was that it is faster than the
traditional GNU linker, ld.bfd — and shorter build times are always
welcome by developers.

While unfortunately ld.gold is essentially unmaintained, we can still
try to use ld.lld when present. Not only it has matured plenty to be
a drop-in replacement, it is well maintained and even faster than ld.gold

As ld.lld is already part of the Flatpak SDK, the only thing missing is
to try to make CMake builds use it by default. Currently it is picked if
the LTO_MODE option is set:

  # Use ld.lld when building with LTO
  CMAKE_DEPENDENT_OPTION(USE_LD_LLD "Use LLD linker" ON
                         "LTO_MODE;NOT USE_LD_GOLD;NOT WIN32" OFF)
Comment 1 Adrian Perez 2022-02-10 04:37:10 PST
For the record, I did some numbers while checking the Mold linker, you
can read here: https://bugs.webkit.org/show_bug.cgi?id=236362#c7

The TL;DR is that LLD is about 4.00x faster than the BFD linker, and
even slightly faster (1.12x) than Gold.
Comment 2 Adrian Perez 2022-02-10 07:10:32 PST
Created attachment 451527 [details]
Patch
Comment 3 Michael Catanzaro 2022-02-10 08:09:19 PST
Comment on attachment 451527 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451527&action=review

> Source/cmake/OptionsCommon.cmake:30
> +    if (CMAKE_BUILD_TYPE STREQUAL Debug)

What makes debug builds special? I guess you cannot enable it for RelWithDebInfo builds because it doesn't support LTO and that would risk trouble with distro builds... but this still seems weird to distinguish between RelWithDebInfo and Debug like this. Maybe it would make more sense to check for DEVELOPER_MODE instead?
Comment 4 Adrian Perez 2022-02-10 11:05:02 PST
(In reply to Michael Catanzaro from comment #3)
> Comment on attachment 451527 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=451527&action=review
> 
> > Source/cmake/OptionsCommon.cmake:30
> > +    if (CMAKE_BUILD_TYPE STREQUAL Debug)
> 
> What makes debug builds special? I guess you cannot enable it for
> RelWithDebInfo builds because it doesn't support LTO and that would risk
> trouble with distro builds... but this still seems weird to distinguish
> between RelWithDebInfo and Debug like this. Maybe it would make more sense
> to check for DEVELOPER_MODE instead?

The LLD linker can do LTO just fine, I routinely do Clang+LLD builds here.
I was trying to think of speeding up the workflow of developers working on
WebKit because every minute counts there, but a few minutes more in a build
for distro package don't matter as much.

You are right and indeed checking for DEVELOPER_MODE makes more sense for
what I am trying to do here — a developer using “build-webkit --release”
will also want a faster edit-compile-test cycle!
Comment 5 Michael Catanzaro 2022-02-10 11:18:11 PST
(In reply to Adrian Perez from comment #4)
> The LLD linker can do LTO just fine, I routinely do Clang+LLD builds here.

Sorry, I confused it with mold. So many linkers.
Comment 6 Michael Catanzaro 2022-02-10 11:18:35 PST
Comment on attachment 451527 [details]
Patch

Seems we agree to check DEVELOPER_MODE instead.
Comment 7 Adrian Perez 2022-02-10 11:26:22 PST
Created attachment 451582 [details]
Patch v2
Comment 8 EWS 2022-02-11 01:18:09 PST
Committed r289611 (247124@main): <https://commits.webkit.org/247124@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451582 [details].