Bug 232938

Summary: [CMake] Reduce memory usage when not using the Gold linker
Product: WebKit Reporter: Alexander Kanavin <alex.kanavin>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WORKSFORME    
Severity: Normal CC: annulen, aperez, bugs-noreply, ews-watchlist, gyuyoung.kim, mcatanzaro, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch aperez: review-, ews-feeder: commit-queue-

Description Alexander Kanavin 2021-11-10 03:29:43 PST
Reduce memory usage when not using the Gold linker
Comment 1 Alexander Kanavin 2021-11-10 03:30:04 PST
Created attachment 443794 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2021-11-17 03:30:23 PST
<rdar://problem/85497847>
Comment 3 Michael Catanzaro 2022-02-16 13:03:22 PST
This probably made sense to do at the time you wrote this patch, but unluckily for you Adrian refactored all of this code a week or two ago. He will probably want to review this.
Comment 4 Adrian Perez 2022-02-16 13:50:48 PST
Comment on attachment 443794 [details]
Patch

In general I do not like this patch because it's going to make the build
process slower for everybody, even for people with plenty of memory to
spare which do not really need to use this linker option. From the ld man
page:

  “This option reduces memory requirements at ld runtime, at the expense
   of linking speed.”

Making linking slower is something that we definitely *not* want in general,
because who spends more time building (and linking) WebKit are developers.
We would rather optimize by default to have shorter edit-compile-test cycles.
Coincidentally, that's the reason why developer builds use LLD if available:
it gets the job done faster.

If you have some *particular* setup in which this is needed, you can specify
the option as part of the LDFLAGS environment variable, and CMake will apply
it for you:

   % LDFLAGS="-Wl,--reduce-memory-overheads" cmake ...

or, if you are using the “build-webkit” script for development:

   % LDFLAGS="-Wl,--reduce-memory-overheads" Tools/Scripts/build-webkit ...

If there is some reason why this is not an option for you, please do explain
and we can reconsider adding something to the WebKit build system, and make it
opt-in — but I would prefer to avoid hardcoding options in our build system
for one-off configurations which typically can be done as suggested above :)


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

> Source/cmake/OptionsCommon.cmake:107
> +    set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--reduce-memory-overheads")

This has changed quite a bit, so currently you should check the LD_VARIANT
variable to only add these options when using the BFD linker. Like this:

  if (LD_VARIANT STREQUAL BFD)
      # Do things.
  endif ()

This is missing adding the option to CMAKE_MODULE_LINKER_FLAGS, too.
Comment 5 Alexander Kanavin 2022-02-16 13:52:58 PST
No problem. This one wasn't actually written by me, I only collected downstream yocto patches, and submitted them all.

I'll drop this on the next version update, and ask the author to upstream first, if they still care.
Comment 6 Adrian Perez 2022-02-16 13:54:13 PST
Ah, one last comment: if you are short on memory in the machine where
you need to build WebKit, you may want to try LLD, which in general is
the fastest linker *and* it uses less memory than Gold. Currently the
Gold linker is abandonware and is mostly unmaintained.
Comment 7 Adrian Perez 2022-02-16 13:57:39 PST
(In reply to Alexander Kanavin from comment #5)
> No problem. This one wasn't actually written by me, I only collected
> downstream yocto patches, and submitted them all.
> 
> I'll drop this on the next version update, and ask the author to upstream
> first, if they still care.

Sounds good. As said, if there is a good reason for a similar patch
we can reconsider; but it seemed rather specific to a particular build
environment.

I definitely don't want to demotivate anybody to bring downstream patches
into the WebKit repository — on the contrary, we are willing to try to
help including patches that help packagers, usually those are generic
(or can be made generic), to benefit everybody :)
Comment 8 Adrian Perez 2022-02-16 13:59:03 PST
I'll close this for now, but again I'll say it's fine to reopen it
if needed.
Comment 9 Alexander Kanavin 2022-02-16 14:00:23 PST
Just for reference, this is where it was added:
https://git.yoctoproject.org/poky/commit/?h=master-next&id=c61948e9b80c664ea298b9f9e4daf3ccec145449

There's a link to a debian ticket too in there.