Bug 137953 – [CMake] Use ld.gold if it is available to speedup builds

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

Now the incremental build takes 1m51s for me if I touch a random cpp in WebCore.

There are 2 reasons for it:
- CMake always run - bug137949
- There are 87 relinking: (+ 5 generator + 1 compile + 1 symlink genarating)
[6/94] Linking CXX static library lib/libwebcore_efl.a
[8/94] Linking CXX shared library lib/libewebkit2.so.1.11.0
[10/94] Linking CXX executable bin/NetworkProcess
[11/94] Linking CXX executable bin/PluginProcess
[12/94] Linking CXX shared library lib/libewebkit_extension_manager.so
[13/94] Linking CXX executable bin/WebProcess
[14/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_application_cache_manager
[15/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_auth_request
[16/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_back_forward_list
[17/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_color_picker
[18/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_context
[19/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_context_history_callbacks
[20/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_cookie_manager
[21/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_database_manager
[22/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_eina_shared_string
[23/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/CloseThenTerminate
[24/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_favicon_database
[25/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_file_chooser_request
[26/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_object
[27/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_page_group
[28/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_settings
[29/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_popup_menu
[30/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_ssl
[31/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_storage_manager
[32/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_view
[33/94] Linking CXX executable bin/TestWebKitAPI/EWebKit2/test_ewk2_window_features
[34/94] Linking CXX shared library lib/libTestRunnerInjectedBundle.so
[35/94] Linking CXX executable bin/WebKitTestRunner
[36/94] Linking CXX executable bin/MiniBrowser
[37/94] Linking CXX executable bin/CanHandleRequest
[38/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/AboutBlankLoad
[39/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/CookieManager
[40/94] Linking CXX executable bin/ResizeReversePaginatedWebView
[41/94] Linking CXX executable bin/DOMWindowExtensionBasic
[42/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/DOMWindowExtensionNoCache
[43/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/DidAssociateFormControls
[44/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/DocumentStartUserScriptAlertCrash
[45/94] Linking CXX executable bin/DownloadDecideDestinationCrash
[46/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/EvaluateJavaScript
[47/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/FailedLoad
[48/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/Find
[49/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/FrameMIMETypeHTML
[50/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/ForceRepaint
[51/94] Linking CXX executable bin/Geolocation
[52/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/FrameMIMETypePNG
[53/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/GetInjectedBundleInitializationUserDataCallback
[54/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/HitTestResultNodeHandle
[55/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/InjectedBundleBasic
[56/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/InjectedBundleFrameHitTest
[57/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/InjectedBundleInitializationUserDataCallbackWins
[58/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/LoadAlternateHTMLStringWithNonDirectoryURL
[59/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/LoadCanceledNoServerRedirectCallback
[60/94] Linking CXX executable bin/TestWebKitAPI/WebCore/LayoutUnit
[61/94] Linking CXX executable bin/LoadPageOnCrash
[62/94] Linking CXX executable bin/NewFirstVisuallyNonEmptyLayoutFrames
[63/94] Linking CXX executable bin/MouseMoveAfterCrash
[64/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/NewFirstVisuallyNonEmptyLayout
[65/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/NewFirstVisuallyNonEmptyLayoutFails
[66/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/NewFirstVisuallyNonEmptyLayoutForImages
[67/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/PageLoadBasic
[68/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/PageLoadDidChangeLocationWithinPageForFrame
[69/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/PreventEmptyUserAgent
[70/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/ParentFrame
[71/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/PrivateBrowsingPushStateNoHistoryCallback
[72/94] Linking CXX executable bin/ReloadPageAfterCrash
[73/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/ShouldGoToBackForwardListItem
[74/94] Linking CXX executable bin/ResizeWindowAfterCrash
[75/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/ResponsivenessTimerDoesntFireEarly
[76/94] Linking CXX executable bin/RestoreSessionStateContainingFormData
[77/94] Linking CXX executable bin/ScrollPinningBehaviors
[78/94] Linking CXX executable bin/TestWebKitAPI/JavaScriptCore/TestJavaScriptCore
[79/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/TerminateTwice
[80/94] Linking CXX executable bin/TestWebKitAPI/WTF/TestWTF
[81/94] Linking CXX shared library lib/libTestWebKitAPIInjectedBundle.so
[82/94] Linking CXX executable bin/TestWebKitAPI/WebCore/URL
[83/94] Linking CXX executable bin/UserMessage
[84/94] Linking CXX executable bin/WKPageGetScaleFactorNotZero
[85/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKPreferences
[86/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKStringJSString
[87/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKURL
[88/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKString
[89/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKViewScrollTo
[90/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKViewClientWebProcessCallbacks
[91/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKViewRestoreZoomAndScrollBackForward
[92/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WKViewUserViewportToContents
[93/94] Linking CXX executable bin/WillLoad
[94/94] Linking CXX executable bin/TestWebKitAPI/WebKit2/WillSendSubmitEvent

This bug is to speedup relinking with using ld.gold instead of the very slow ld.

[Csaba Osztrogonác]

Using ld.gold decreased the incremental build time from 1m51s to 50s 
when CMake always runs.

After applying the optimization from bug137949 not to run CMake
always, the build time is only 22s instead of the original 1m51s.

[Csaba Osztrogonác]

Created attachment 240267 [details]
WIP patch

WIP patch, added a platform independent USE_GOLD_LINKER option, disabled by default. It can be enabled with build-webkit --cmakeargs="-DUSE_GOLD_LINKER=ON"

[Csaba Osztrogonác]

cc-ing GTK folks too, maybe you are interested in it too. 
(The patch is based on your great DEBUG_FISSION option)

The question is if we should make using ld.gold default or not.
In this case I meant it can be disabled manually or the config
could disable it automatically if it isn't available. (with or
without warning the developer)

Or do you think if using gold linker should be disabled by default 
for some reason, and should be an opt-in option?

[Carlos Alberto Lopez Perez]

(In reply to comment #3)
> cc-ing GTK folks too, maybe you are interested in it too. 
> (The patch is based on your great DEBUG_FISSION option)
> 
> The question is if we should make using ld.gold default or not.
> In this case I meant it can be disabled manually or the config
> could disable it automatically if it isn't available. (with or
> without warning the developer)
> 
> Or do you think if using gold linker should be disabled by default 
> for some reason, and should be an opt-in option?

We are using gold on the GTK buildbots, but we resort to the path trick:

export PATH="/usr/lib/gold-ld:${PATH}"

I never heard before of this "-fuse-ld=gold" parameter. Is that option supported upstream? because after a bit of googling it seems to be a debian/ubuntu only thing. So perhaps that can break non debian-based (fedora/arch/suse/tizen..). Check: https://bugs.webkit.org/show_bug.cgi?id=89312

[Alberto Garcia]

(In reply to comment #4)
> I never heard before of this "-fuse-ld=gold" parameter.

It seems to be a normal GCC option:

https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html

I don't know when it was introduced, though. Probably not long ago.

I don't have anything against using gold by default if available as
long as it doesn't break the build with one of our supported versions
of gcc.

Then again it's also something that users can set up themselves easily
in their build environments (in fact that's what I do), so ...

[Csaba Osztrogonác]

I checked, it is available in upstream GCC too from 4.8.0:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55470

WebKit requires at least 4.7:
http://trac.webkit.org/browser/trunk/Source/WTF/wtf/Compiler.h?order=name#L68

But it isn't a problem, because the cmake script can check if fuse-ls=gold GCC
option is available or not. If no, we simply don't add it to C(XX)FLAGS.

[Csaba Osztrogonác]

Created attachment 241101 [details]
Patch

I think we should use ld.gold if it is available, and disable it only if it isn't available.

[WebKit Commit Bot]

Attachment 241101 [details] did not pass style-queue:


ERROR: Source/cmake/OptionsCommon.cmake:52:  One space between command "else" and its parentheses, should be "else ("  [whitespace/parentheses] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Csaba Osztrogonác]

Created attachment 241102 [details]
Patch

style fixed

[Csaba Osztrogonác]

any opinion? pros/cons?

[Csaba Osztrogonác]

ping?

[Alberto Garcia]

Hey, I don't really have a strong opinion about the idea of using gold
by default. As I said earlier, choosing which linker to use is already
easy enough

My question is: how would you disable it with your patch? Is there an
easy way to use the GNU linker even if gold is available?

[Csaba Osztrogonác]

(In reply to comment #12)
> Hey, I don't really have a strong opinion about the idea of using gold
> by default. As I said earlier, choosing which linker to use is already
> easy enough

Yes, it is easy, you have to copy ld.gold somewhere else
with ld name and put it in the path before the normal ld.

Why not use it by default everywhere without local hacking?
Now ld.bfd is the default and I would like to change it to ld.gold.

> My question is: how would you disable it with your patch? Is there an
> easy way to use the GNU linker even if gold is available?

Good point, it isn't trivial with the attached patch. I'll 
update it immediately to let folks use the good old linker too.

[Csaba Osztrogonác]

Created attachment 241944 [details]
Patch

Now ld.gold is used if it is available and isn't disabled explicitly with USE_LD_GOLD=OFF option. Additionally I removed the redundant check from DEBUG_FISSION code path.

[Alberto Garcia]

(In reply to comment #13)
> > choosing which linker to use is already easy enough
> Yes, it is easy, you have to copy ld.gold somewhere else with ld
> name and put it in the path before the normal ld.

You don't really need to copy anything:

$ PATH=/usr/lib/gold-ld:$PATH Tools/Scripts/build-webkit ...

> Good point, it isn't trivial with the attached patch. I'll update it
> immediately to let folks use the good old linker too.

This patch looks good to me, thanks!

[Csaba Osztrogonác]

(In reply to comment #15)
> You don't really need to copy anything:
> $ PATH=/usr/lib/gold-ld:$PATH Tools/Scripts/build-webkit ...

Good point, I didn't realize that gold-ld directory is installed by default. :)

[Carlos Garcia Campos]

Comment on attachment 241944 [details]
Patch

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

> ChangeLog:3
> +        [EFL] Use ld.gold if it is available to speedup builds

Please, remove the EFL prefix from the ChangeLog and commit message before landing this.

[Csaba Osztrogonác]

Committed r176442: <http://trac.webkit.org/changeset/176442>