Bug 77327 - [CMAKE] Fix build break because of memory exhausted.
Summary: [CMAKE] Fix build break because of memory exhausted.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-30 04:09 PST by Ryuan Choi
Modified: 2012-09-18 00:19 PDT (History)
11 users (show)

See Also:


Attachments
Patch (13.41 KB, patch)
2012-01-31 02:37 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.45 KB, patch)
2012-01-31 02:45 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.43 KB, patch)
2012-01-31 03:30 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (13.40 KB, patch)
2012-02-06 18:46 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
anotherApproach (1.92 KB, patch)
2012-03-27 03:54 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
anotherApproach2 (1.56 KB, patch)
2012-03-27 17:08 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
anotherApproach3 (1.57 KB, patch)
2012-03-27 17:35 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (2.53 KB, patch)
2012-09-12 01:34 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (2.51 KB, patch)
2012-09-12 01:49 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (2.50 KB, patch)
2012-09-12 01:56 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.47 KB, patch)
2012-09-12 03:52 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.61 KB, patch)
2012-09-12 04:38 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.58 KB, patch)
2012-09-17 23:49 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.