232938 2021-11-10 03:29:43 -0800 [CMake] Reduce memory usage when not using the Gold linker 2022-02-16 14:00:23 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED WORKSFORME InRadar P2 Normal --- 1 alex.kanavin webkit-unassigned annulen aperez bugs-noreply ews-watchlist gyuyoung.kim mcatanzaro ryuan.choi sergio webkit-bug-importer oldest_to_newest 1813536 0 alex.kanavin 2021-11-10 03:29:43 -0800 Reduce memory usage when not using the Gold linker 1813537 1 443794 alex.kanavin 2021-11-10 03:30:04 -0800 Created attachment 443794 Patch 1815889 2 webkit-bug-importer 2021-11-17 03:30:23 -0800 <rdar://problem/85497847> 1842475 3 mcatanzaro 2022-02-16 13:03:22 -0800 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. 1842507 4 443794 aperez 2022-02-16 13:50:48 -0800 Comment on attachment 443794 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. 1842510 5 alex.kanavin 2022-02-16 13:52:58 -0800 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. 1842512 6 aperez 2022-02-16 13:54:13 -0800 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. 1842513 7 aperez 2022-02-16 13:57:39 -0800 (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 :) 1842515 8 aperez 2022-02-16 13:59:03 -0800 I'll close this for now, but again I'll say it's fine to reopen it if needed. 1842516 9 alex.kanavin 2022-02-16 14:00:23 -0800 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. 443794 2021-11-10 03:30:04 -0800 2022-02-16 13:50:48 -0800 Patch bug-232938-20211110123003.patch text/plain 1368 alex.kanavin U3VidmVyc2lvbiBSZXZpc2lvbjogMjg1NTcwCmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvT3B0 aW9uc0NvbW1vbi5jbWFrZSBiL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCmluZGV4 IDZjNDBmMjM4YWI3N2RjZGJhZDE1NzA0N2QxODY0ZjU0NTlmNjIwM2MuLjcxOGQzMGE0ODA1Y2Nj YzUzM2FiZTU4ZGI2YTdhMzIxYjQ0Zjk0NTAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9jbWFrZS9PcHRp b25zQ29tbW9uLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCkBA IC0xMDEsNiArMTAxLDExIEBAIG9wdGlvbihHQ0NfT0ZGTElORUFTTV9TT1VSQ0VfTUFQCiAgICR7 R0NDX09GRkxJTkVBU01fU09VUkNFX01BUF9ERUZBVUxUfSkKIAogb3B0aW9uKFVTRV9BUFBMRV9J Q1UgIlVzZSBBcHBsZSdzIGludGVybmFsIElDVSIgJHtBUFBMRX0pCisjIFBhc3MgLS1yZWR1Y2Ut bWVtb3J5LW92ZXJoZWFkcyB0byB0aGUgYmZkIGxpbmtlciBpbiBvcmRlciB0byBzYXZlIG1lbW9y eQoraWYgKE5PVCBVU0VfTERfR09MRCkKKyAgICBzZXQoQ01BS0VfRVhFX0xJTktFUl9GTEFHUyAi JHtDTUFLRV9FWEVfTElOS0VSX0ZMQUdTfSAtV2wsLS1yZWR1Y2UtbWVtb3J5LW92ZXJoZWFkcyIp CisgICAgc2V0KENNQUtFX1NIQVJFRF9MSU5LRVJfRkxBR1MgIiR7Q01BS0VfRVhFX0xJTktFUl9G TEFHU30gLVdsLC0tcmVkdWNlLW1lbW9yeS1vdmVyaGVhZHMiKQorZW5kaWYgKCkKIAogIyBFbmFi bGUgdGhlIHVzYWdlIG9mIE9wZW5NUC4KICMgIC0gQXQgdGhpcyBtb21lbnQsIE9wZW5NUCBpcyBv bmx5IHVzZWQgYXMgYW4gYWx0ZXJuYXRpdmUgaW1wbGVtZW50YXRpb24KZGlmZiAtLWdpdCBhL0No YW5nZUxvZyBiL0NoYW5nZUxvZwppbmRleCAzZmE2NDE4MWYzNDc5ZDYxZDRiMjVkZjk0Yjk3MjVh MzI0NTI3YTVlLi4zZTAzNjk1MmU4YjdlY2RlMTI1YTcyY2NhYjQwNGQ5OGUzNTg4N2ExIDEwMDY0 NAotLS0gYS9DaGFuZ2VMb2cKKysrIGIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAKKzIwMjEt MTEtMTAgIEFsZXhhbmRlciBLYW5hdmluICA8YWxleEBsaW51dHJvbml4LmRlPgorCisgICAgICAg IFJlZHVjZSBtZW1vcnkgdXNhZ2Ugd2hlbiBub3QgdXNpbmcgdGhlIEdvbGQgbGlua2VyCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzI5MzgKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNvdXJjZS9jbWFr ZS9PcHRpb25zQ29tbW9uLmNtYWtlOgorCiAyMDIxLTExLTA5ICBKIFBhc2NvZSAgPGpfcGFzY29l QGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgal9wYXNjb2UgdG8gY29udHJpYnV0b3JzLmpzb24K