Bug 137953 - [CMake] Use ld.gold if it is available to speedup builds
Summary: [CMake] Use ld.gold if it is available to speedup builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Csaba Osztrogonác
URL:
Keywords:
Depends on:
Blocks: 137818
  Show dependency treegraph
 
Reported: 2014-10-22 04:36 PDT by Csaba Osztrogonác
Modified: 2014-11-20 23:07 PST (History)
10 users (show)

See Also:


Attachments
WIP patch (1.64 KB, patch)
2014-10-22 05:11 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.02 KB, patch)
2014-11-06 05:48 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.03 KB, patch)
2014-11-06 05:53 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff
Patch (2.22 KB, patch)
2014-11-20 06:39 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2014-10-22 04:36:18 PDT
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.
Comment 1 Csaba Osztrogonác 2014-10-22 05:03:33 PDT
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.
Comment 2 Csaba Osztrogonác 2014-10-22 05:11:55 PDT
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"
Comment 3 Csaba Osztrogonác 2014-10-22 05:19:13 PDT
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?
Comment 4 Carlos Alberto Lopez Perez 2014-10-22 05:36:40 PDT
(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
Comment 5 Alberto Garcia 2014-10-22 05:59:20 PDT
(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 ...
Comment 6 Csaba Osztrogonác 2014-10-22 05:59:44 PDT
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.
Comment 7 Csaba Osztrogonác 2014-11-06 05:48:30 PST
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.
Comment 8 WebKit Commit Bot 2014-11-06 05:51:29 PST
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.
Comment 9 Csaba Osztrogonác 2014-11-06 05:53:07 PST
Created attachment 241102 [details]
Patch

style fixed
Comment 10 Csaba Osztrogonác 2014-11-11 23:42:06 PST
any opinion? pros/cons?
Comment 11 Csaba Osztrogonác 2014-11-19 14:01:26 PST
ping?
Comment 12 Alberto Garcia 2014-11-20 05:40:22 PST
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?
Comment 13 Csaba Osztrogonác 2014-11-20 06:35:17 PST
(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.
Comment 14 Csaba Osztrogonác 2014-11-20 06:39:08 PST
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.
Comment 15 Alberto Garcia 2014-11-20 07:03:59 PST
(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!
Comment 16 Csaba Osztrogonác 2014-11-20 07:07:33 PST
(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. :)
Comment 17 Carlos Garcia Campos 2014-11-20 22:52:26 PST
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.
Comment 18 Csaba Osztrogonác 2014-11-20 23:06:24 PST
Committed r176442: <http://trac.webkit.org/changeset/176442>