Bug 137953

Summary: [CMake] Use ld.gold if it is available to speedup builds
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: Tools / TestsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, cgarcia, clopez, commit-queue, gyuyoung.kim, mrobinson, ossy, rakuco, ryuan.choi, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 137818    
Attachments:
Description Flags
WIP patch
none
Patch
none
Patch
none
Patch none

Csaba Osztrogonác
Reported 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.
Attachments
WIP patch (1.64 KB, patch)
2014-10-22 05:11 PDT, Csaba Osztrogonác
no flags
Patch (2.02 KB, patch)
2014-11-06 05:48 PST, Csaba Osztrogonác
no flags
Patch (2.03 KB, patch)
2014-11-06 05:53 PST, Csaba Osztrogonác
no flags
Patch (2.22 KB, patch)
2014-11-20 06:39 PST, Csaba Osztrogonác
no flags
Csaba Osztrogonác
Comment 1 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.
Csaba Osztrogonác
Comment 2 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"
Csaba Osztrogonác
Comment 3 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?
Carlos Alberto Lopez Perez
Comment 4 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
Alberto Garcia
Comment 5 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 ...
Csaba Osztrogonác
Comment 6 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.
Csaba Osztrogonác
Comment 7 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.
WebKit Commit Bot
Comment 8 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.
Csaba Osztrogonác
Comment 9 2014-11-06 05:53:07 PST
Created attachment 241102 [details] Patch style fixed
Csaba Osztrogonác
Comment 10 2014-11-11 23:42:06 PST
any opinion? pros/cons?
Csaba Osztrogonác
Comment 11 2014-11-19 14:01:26 PST
ping?
Alberto Garcia
Comment 12 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?
Csaba Osztrogonác
Comment 13 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.
Csaba Osztrogonác
Comment 14 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.
Alberto Garcia
Comment 15 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!
Csaba Osztrogonác
Comment 16 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. :)
Carlos Garcia Campos
Comment 17 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.
Csaba Osztrogonác
Comment 18 2014-11-20 23:06:24 PST
Note You need to log in before you can comment on or make changes to this bug.