[CMake] Use JOB_POOLS to avoid memory-hungry linker processes running at the same time

[Adrian Perez]

Ninja supports the so called “job pools”, to which targets can be
assigned. A job pool allows limiting the amount of parallel jobs
that will run in parallel:

  https://ninja-build.org/manual.html#ref_pool

CMake supports configuring job pools, even when they are only used
by the Ninja build backend:

  https://cmake.org/cmake/help/v3.3/prop_gbl/JOB_POOLS.html

Using this we could limit the amount of parallel linker processes
which are run at the same time. This is particularly important to
avoid OOM situations when linking in non-release builds.

My proposal is doing something like the following in our CMake
build files:

  set_property(GLOBAL PROPERTY JOB_POOLS
               release_link_jobs=4
               debug_link_jobs=2)

  if (${CMAKE_BUILD_TYPE} STREQUAL "Release")
      set(CMAKE_JOB_POOL_LINK release_link_jobs)
  else ()
      set(CMAKE_JOB_POOL_LINK debug_link_jobs)
  endif ()

My tests in a build machine which hosts an EWS and that I use also
for development (so there are sometimes two debug builds running
in parallel) show that the above would avoid hitting OOM situations.

Opinions?

[Michael Catanzaro]

OK

[Adrian Perez]

Created attachment 344111 [details]
Patch

[Adrian Perez]

FTR, I am running a couple of fresh builds to compare built
times with/without the patch before setting the cq? flag.

[Adrian Perez]

Created attachment 344112 [details]
Patch

Fixed patch. Now really using 4/2 for release/debug builds.

[Michael Catanzaro]

Comment on attachment 344112 [details]
Patch

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

> Source/cmake/WebKitCommon.cmake:66
> +    if (${CMAKE_BUILD_TYPE} STREQUAL "Release")

It fails for MinSizeRel, for example. But we can't test for everything, and when the test fails you pick the safe option, so it's good IMO.

[Adrian Perez]

(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 344112 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344112&action=review
> 
> > Source/cmake/WebKitCommon.cmake:66
> > +    if (${CMAKE_BUILD_TYPE} STREQUAL "Release")
> 
> It fails for MinSizeRel, for example. But we can't test for everything, and
> when the test fails you pick the safe option, so it's good IMO.

Good catch, I'll add a check for MinSizeRel as well.

[Adrian Perez]

Created attachment 344133 [details]
Patch

Handle also MinSizeRel builds.

The build times with and without the patch are roughly the same in my build
machine, so this seems quite safe to land.

[Michael Catanzaro]

Comment on attachment 344133 [details]
Patch

Looks like the GTK EWS died:

ICECC[3965] 14:15:38: flush_writebuf() failed Resource temporarily unavailable
ICECC[3965] 14:15:40: write of source chunk to host 192.168.10.42
ICECC[3965] 14:15:40: failed  Resource temporarily unavailable
ICECC[3965] 14:15:40: got exception Error 15 - write to host failed (192.168.10.42)

[EWS Watchlist]

Comment on attachment 344133 [details]
Patch

Attachment 344133 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8417088

New failing tests:
http/tests/security/local-video-source-from-remote.html

[EWS Watchlist]

Created attachment 344145 [details]
Archive of layout-test-results from ews200 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews200  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Adrian Perez]

(In reply to Michael Catanzaro from comment #8)
> Comment on attachment 344133 [details]
> Patch
> 
> Looks like the GTK EWS died:
> 
> ICECC[3965] 14:15:38: flush_writebuf() failed Resource temporarily
> unavailable
> ICECC[3965] 14:15:40: write of source chunk to host 192.168.10.42
> ICECC[3965] 14:15:40: failed  Resource temporarily unavailable
> ICECC[3965] 14:15:40: got exception Error 15 - write to host failed
> (192.168.10.42)

It seems like a temporary failure, I see other patches have passed
looking into page for this EWS bot:

  https://webkit-queues.webkit.org/queue-status/gtk-wk2-ews/bots/igalia4-gtk-wk2-ews

[Adrian Perez]

Created attachment 344152 [details]
Patch

Re-uploading to make the GTK+ EWS go over this again

[WebKit Commit Bot]

Comment on attachment 344152 [details]
Patch

Clearing flags on attachment: 344152

Committed r233454: <https://trac.webkit.org/changeset/233454>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41750773>