Bug 77327

Summary: [CMAKE] Fix build break because of memory exhausted.
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: dbates, gyuyoung.kim, gyuyoung.kim, joone, lucas.de.marchi, mxie, paroga, rakuco, rwlbuis, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
anotherApproach
none
anotherApproach2
none
anotherApproach3
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Ryuan Choi 2012-01-30 04:09:39 PST
Linking is failed in debug build.

I used "./Tools/Scripts/build-webkit --efl --debug" and get below error.

Linking CXX executable ../../../Programs/DumpRenderTree
/usr/bin/ld: final link failed: Memory exhausted
collect2: ld returned 1 exit status
make[2]: *** [Programs/DumpRenderTree] 오류 1
make[1]: *** [Tools/DumpRenderTree/efl/CMakeFiles/Programs/DumpRenderTree.dir/all] 오류 2
make: *** [all] 오류 2

I think that we need some tricks like Qt port did.
Comment 1 Ryuan Choi 2012-01-31 02:37:32 PST
Created attachment 124691 [details]
Patch
Comment 2 WebKit Review Bot 2012-01-31 02:39:01 PST
Attachment 124691 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ryuan Choi 2012-01-31 02:45:05 PST
Created attachment 124692 [details]
Patch
Comment 4 WebKit Review Bot 2012-01-31 02:48:28 PST
Attachment 124692 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   3b4a9fa..2a654fd  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 106348 = 3b4a9fa133678770a77bc6d602361e3cd4cb2dd1
r106349 = 976b1826441f3aef8b3a6be4636b2bbcb98365cc
r106350 = 2a654fd8513245efac73e1f8526121f158404c41
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Gyuyoung Kim 2012-01-31 02:54:28 PST
Comment on attachment 124692 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Use SVGAllInOne files to avoid memory exhaustion for debug builds on linux

I don't understand well what do you mean in this changelog.
Comment 6 Ryuan Choi 2012-01-31 03:30:29 PST
Created attachment 124700 [details]
Patch
Comment 7 WebKit Review Bot 2012-01-31 03:33:12 PST
Attachment 124700 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Ryuan Choi 2012-01-31 03:37:47 PST
(In reply to comment #5)
> (From update of attachment 124692 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124692&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Use SVGAllInOne files to avoid memory exhaustion for debug builds on linux
> 
> I don't understand well what do you mean in this changelog.

little bit changed.

BTW, I don't know why style-queue failed to merge. :(
Comment 9 Ryuan Choi 2012-02-06 18:46:41 PST
Created attachment 125751 [details]
Patch
Comment 10 Ryuan Choi 2012-02-06 18:47:30 PST
(In reply to comment #9)
> Created an attachment (id=125751) [details]
> Patch

rebased for green style bot.
Comment 11 Raphael Kubo da Costa (:rakuco) 2012-02-07 04:13:28 PST
It looks like a very hackish solution tuned to work in a very specific environment. The right solution would be to just get more RAM :)

Building webkit in debug mode is a very heavy task, but it still works here in a 4GB machine.
Comment 12 Antonio Gomes 2012-02-07 11:09:36 PST
guys, we should keep three eyes at it:)
Comment 13 Raphael Kubo da Costa (:rakuco) 2012-02-09 07:39:51 PST
Comment on attachment 125751 [details]
Patch

r-'ing the currently proposed patch.
Comment 14 Ryuan Choi 2012-02-27 00:29:52 PST
(In reply to comment #11)
> It looks like a very hackish solution tuned to work in a very specific environment. The right solution would be to just get more RAM :)
> 
> Building webkit in debug mode is a very heavy task, but it still works here in a 4GB machine.

OK, I still used this to debug, but I agree that this is little bit hackish.

Anyway, I also used 4GB RAM.
Comment 15 Ryuan Choi 2012-03-27 03:54:43 PDT
Reopening to attach new patch.
Comment 16 Ryuan Choi 2012-03-27 03:54:49 PDT
Created attachment 134016 [details]
anotherApproach
Comment 17 Ryuan Choi 2012-03-27 04:08:42 PDT
(In reply to comment #16)
> Created an attachment (id=134016) [details]
> anotherApproach

I tried different approach after reading https://lists.webkit.org/pipermail/webkit-dev/2012-March/020111.html
Comment 18 Raphael Kubo da Costa (:rakuco) 2012-03-27 07:54:18 PDT
Comment on attachment 134016 [details]
anotherApproach

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

> Source/cmake/WebKitHelpers.cmake:61
> +    IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> +        GET_TARGET_PROPERTY(OLD_LINK_FLAGS ${_target} LINK_FLAGS)
> +
> +        IF (WTF_CPU_X86)
> +            SET(OLD_LINK_FLAGS "${OLD_LINK_FLAGS} -Wl,--no-keep-memory")
> +        ENDIF ()
> +
> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES
> +            LINK_FLAGS "${OLD_LINK_FLAGS}")
> +    ENDIF ()

You could rewrite this as
  IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86)
      # Update target properties
  ENDIF ()

It would be good to add a comment explaining why this is being done, perhaps even pointing to the mentioned link.
Comment 19 Raphael Kubo da Costa (:rakuco) 2012-03-27 07:54:57 PDT
BTW, are you still unable to link WebCore on your machine?
Comment 20 Ryuan Choi 2012-03-27 17:08:38 PDT
Created attachment 134174 [details]
anotherApproach2
Comment 21 Ryuan Choi 2012-03-27 17:15:27 PDT
(In reply to comment #18)
> (From update of attachment 134016 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134016&action=review
> 
> > Source/cmake/WebKitHelpers.cmake:61
> > +    IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
> > +        GET_TARGET_PROPERTY(OLD_LINK_FLAGS ${_target} LINK_FLAGS)
> > +
> > +        IF (WTF_CPU_X86)
> > +            SET(OLD_LINK_FLAGS "${OLD_LINK_FLAGS} -Wl,--no-keep-memory")
> > +        ENDIF ()
> > +
> > +        SET_TARGET_PROPERTIES (${_target} PROPERTIES
> > +            LINK_FLAGS "${OLD_LINK_FLAGS}")
> > +    ENDIF ()
> 
> You could rewrite this as
>   IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86)
>       # Update target properties
>   ENDIF ()
> 
Done. and checked 'DEBUG' also.

> It would be good to add a comment explaining why this is being done, perhaps even pointing to the mentioned link.
Add some comment.


(In reply to comment #19)
> BTW, are you still unable to link WebCore on your machine?

Yes, so I just disabled SVG when I want debug binary.
and also, my collegue, who test SVG feature, requests this patch.
Comment 22 Raphael Kubo da Costa (:rakuco) 2012-03-27 17:22:30 PDT
Comment on attachment 134174 [details]
anotherApproach2

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

No objections from my side besides the comment below.

> Source/cmake/WebKitHelpers.cmake:52
> +    IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86 AND CMAKE_BUILD_TYPE STREQUAL Debug)

Please enclose Debug within quotes, just like "GNU".
Comment 23 Ryuan Choi 2012-03-27 17:35:09 PDT
Created attachment 134183 [details]
anotherApproach3
Comment 24 Ryuan Choi 2012-03-27 17:36:42 PDT
(In reply to comment #22)
> (From update of attachment 134174 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134174&action=review
> 
> No objections from my side besides the comment below.
> 
> > Source/cmake/WebKitHelpers.cmake:52
> > +    IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86 AND CMAKE_BUILD_TYPE STREQUAL Debug)
> 
> Please enclose Debug within quotes, just like "GNU".

Thanks, I tried like you mentioned.
Comment 25 Ryuan Choi 2012-09-12 01:34:37 PDT
Created attachment 163543 [details]
Patch
Comment 26 Raphael Kubo da Costa (:rakuco) 2012-09-12 01:43:17 PDT
Comment on attachment 163543 [details]
Patch

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

> Source/cmake/WebKitHelpers.cmake:59
> +    IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU"
> +            AND WTF_CPU_X86
> +            AND "${CMAKE_BUILD_TYPE}" STREQUAL "Debug")

You could as well just have put this into a single line.

> Source/cmake/WebKitHelpers.cmake:66
> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES
> +            LINK_FLAGS "${OLD_LINK_FLAGS}")

Ditto.
Comment 27 Ryuan Choi 2012-09-12 01:49:13 PDT
Created attachment 163548 [details]
Patch
Comment 28 Ryuan Choi 2012-09-12 01:56:02 PDT
Created attachment 163550 [details]
Patch
Comment 29 Patrick R. Gansterer 2012-09-12 01:58:04 PDT
Comment on attachment 163550 [details]
Patch

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

> Source/cmake/WebKitHelpers.cmake:63
> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES LINK_FLAGS "${OLD_LINK_FLAGS}")

do not set the linker flags on the target. use the CMAKE_XXX variables instead. If it's not possible then write the reason in the ChangeLog.
Comment 30 Raphael Kubo da Costa (:rakuco) 2012-09-12 03:04:24 PDT
Comment on attachment 163550 [details]
Patch

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

>> Source/cmake/WebKitHelpers.cmake:63
>> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES LINK_FLAGS "${OLD_LINK_FLAGS}")
> 
> do not set the linker flags on the target. use the CMAKE_XXX variables instead. If it's not possible then write the reason in the ChangeLog.

Why?
Comment 31 Patrick R. Gansterer 2012-09-12 03:09:00 PDT
Comment on attachment 163550 [details]
Patch

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

>>> Source/cmake/WebKitHelpers.cmake:63
>>> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES LINK_FLAGS "${OLD_LINK_FLAGS}")
>> 
>> do not set the linker flags on the target. use the CMAKE_XXX variables instead. If it's not possible then write the reason in the ChangeLog.
> 
> Why?

beause linker flags (or at least the the lags in this patch) are sth project-wide and calling WEBKIT_SET_EXTRA_LINKER_FLAGS() for every target is as ugly as the WEBKIT_SET_EXTRA_COMPILER_FLAGS() calls are already
Comment 32 Raphael Kubo da Costa (:rakuco) 2012-09-12 03:29:32 PDT
Comment on attachment 163550 [details]
Patch

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

>>>> Source/cmake/WebKitHelpers.cmake:63
>>>> +        SET_TARGET_PROPERTIES (${_target} PROPERTIES LINK_FLAGS "${OLD_LINK_FLAGS}")
>>> 
>>> do not set the linker flags on the target. use the CMAKE_XXX variables instead. If it's not possible then write the reason in the ChangeLog.
>> 
>> Why?
> 
> beause linker flags (or at least the the lags in this patch) are sth project-wide and calling WEBKIT_SET_EXTRA_LINKER_FLAGS() for every target is as ugly as the WEBKIT_SET_EXTRA_COMPILER_FLAGS() calls are already

I don't really see a problem with setting these properties per target. In this case it wouldn't make much difference anyway. I'm OK with setting this in a project-wide way as well.
Comment 33 Ryuan Choi 2012-09-12 03:52:11 PDT
Created attachment 163575 [details]
Patch
Comment 34 Ryuan Choi 2012-09-12 03:54:14 PDT
(In reply to comment #29)
> (From update of attachment 163550 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163550&action=review
> 
> > Source/cmake/WebKitHelpers.cmake:63
> > +        SET_TARGET_PROPERTIES (${_target} PROPERTIES LINK_FLAGS "${OLD_LINK_FLAGS}")
> 
> do not set the linker flags on the target. use the CMAKE_XXX variables instead. If it's not possible then write the reason in the ChangeLog.

Sorry for late answer. I am compiling. :)

I tried to use CMAKE_SHARED_LINKER_FLAGS.
I hope that it is what you mentioned.


Thank you.
Comment 35 Patrick R. Gansterer 2012-09-12 03:56:13 PDT
Comment on attachment 163575 [details]
Patch

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

> I hope that it is what you mentioned.

Yes, thanks!

> Source/cmake/OptionsCommon.cmake:29
> +IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86 AND "${CMAKE_BUILD_TYPE}" STREQUAL "Debug")

is WTF_CPU_X86 required? imho the WTF_CPU_X86 means the target platform and not the compiler platform. Is there a reason why you add it only to the Debug build? Are there problems with this flags in Release builds?
Comment 36 Ryuan Choi 2012-09-12 04:02:55 PDT
(In reply to comment #35)
> (From update of attachment 163575 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163575&action=review
> 
> > I hope that it is what you mentioned.
> 
> Yes, thanks!
> 
> > Source/cmake/OptionsCommon.cmake:29
> > +IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86 AND "${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
> 
> is WTF_CPU_X86 required? imho the WTF_CPU_X86 means the target platform and not the compiler platform. Is there a reason why you add it only to the Debug build? Are there problems with this flags in Release builds?

Oops, I did not catch it.
If it is not for compiler platform. I think that I should fix it.

And this option has trade off between speed and memory.
So I added it to debug build.
Comment 37 Ryuan Choi 2012-09-12 04:38:30 PDT
Created attachment 163583 [details]
Patch
Comment 38 Ryuan Choi 2012-09-12 04:40:11 PDT
(In reply to comment #35)
> (From update of attachment 163575 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163575&action=review
> 
> > I hope that it is what you mentioned.
> 
> Yes, thanks!
> 
> > Source/cmake/OptionsCommon.cmake:29
> > +IF ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND WTF_CPU_X86 AND "${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
> 
> is WTF_CPU_X86 required? imho the WTF_CPU_X86 means the target platform and not the compiler platform. Is there a reason why you add it only to the Debug build? Are there problems with this flags in Release builds?

Changed to use CMAKE_HOST_SYSTEM_PROCESSOR instead of CMAKE_SYSTEM_PROCESSOR

Thank you.
Comment 39 Ryuan Choi 2012-09-17 23:49:27 PDT
Created attachment 164501 [details]
Patch
Comment 40 Gyuyoung Kim 2012-09-17 23:55:27 PDT
Comment on attachment 164501 [details]
Patch

Looks fine. Patrick also agreed this patch.
Comment 41 WebKit Review Bot 2012-09-18 00:19:42 PDT
Comment on attachment 164501 [details]
Patch

Clearing flags on attachment: 164501

Committed r128855: <http://trac.webkit.org/changeset/128855>
Comment 42 WebKit Review Bot 2012-09-18 00:19:48 PDT
All reviewed patches have been landed.  Closing bug.