Bug 155871 – [Win] Improve CMake build performance

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brent Fulgham]

The time required to build our Windows WebKit builds has increased significantly since we switched to CMake.

While investigating this I noticed that we no longer use the DerivedSources.cpp "all in one" file to drive the build of our Derived Sources. We started using this years ago to address long link times and other build problems, so I don't think we should have stopped using it.

This patch corrects the build as follows:
1. It uses "DerivedSources.cpp"
2. To avoid breaking CMake dependency analysis, we revise the VS projects to show the header for each auto generated file, which forces CMake to make sure they are kept in sync, without causing VS to actually build the files.
3. Updates "DerivedSources.cpp" with some new files that were added since the last time it was updated.

Although its annoying to keep DerivedSources.cpp in sync with the other pieces of the build system, it's no worse than the dozen or so other "all in one" files we already use (on multiple ports). Plus, it's unlikely we will break this in the future because a missing file just means that VS builds it separately, slightly slowing the build. Of course, removing an IDL file without updating the DerivedSources file could break the build, but EWS should show that quite clearly.

[Brent Fulgham]

Created attachment 274884 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/DerivedSources.cpp:349:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/DerivedSources.cpp:378:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/DerivedSources.cpp:412:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/DerivedSources.cpp:439:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebCore/DerivedSources.cpp:590:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 5 in 5 files


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

[Alex Christensen]

Comment on attachment 274884 [details]
Patch

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

r=me

> Source/WebCore/CMakeLists.txt:3805
> +    set(WebCore_SOURCES ${newSources})
> +    list(APPEND WebCore_SOURCES DerivedSources.cpp)

This can be done in one command, like is done with the other call to PROCESS_ALLINONE_FILE

> Source/cmake/WebKitMacros.cmake:374
> +            string(REGEX REPLACE "(.*)\\.cpp" "\\1" _allin_no_ext ${_allin})
> +            string(REGEX REPLACE ";([^;]*/)${_allin_no_ext}\\.cpp;" ";\\1${_allin_no_ext}.h;" _new_result "${${_result_file_list}};")

A comment here would be nice.  This is hard to read and tell what is going on.
# For DerivedSources.cpp, we still need the derived sources to be generated, but we do not want them to be compiled separately, 
# so we add the header to the result file list.

[Brent Fulgham]

Created attachment 274887 [details]
Patch v2

[Brent Fulgham]

(In reply to comment #3)
> Comment on attachment 274884 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=274884&action=review
> 
> r=me
> 
> > Source/WebCore/CMakeLists.txt:3805
> > +    set(WebCore_SOURCES ${newSources})
> > +    list(APPEND WebCore_SOURCES DerivedSources.cpp)
> 
> This can be done in one command, like is done with the other call to
> PROCESS_ALLINONE_FILE

OK!

> > Source/cmake/WebKitMacros.cmake:374
> > +            string(REGEX REPLACE "(.*)\\.cpp" "\\1" _allin_no_ext ${_allin})
> 
> A comment here would be nice.  This is hard to read and tell what is going
> on.

Will do!

[Brent Fulgham]

Committed in r198669. <https://trac.webkit.org/r198669>