Bug 178225 - Xcode build should parallelize targets
Summary: Xcode build should parallelize targets
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-12 12:24 PDT by Keith Miller
Modified: 2022-06-17 17:57 PDT (History)
15 users (show)

See Also:


Attachments
Patch for EWS (107.25 KB, patch)
2020-09-22 17:06 PDT, Andy Estes
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Enable "Parallelize Build" option in WebKit.xcworkspace (110.98 KB, patch)
2020-09-22 18:50 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Enable "Parallelize Build" option in WebKit.xcworkspace (110.98 KB, patch)
2020-09-22 18:58 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Enable "Parallelize Build" option in WebKit.xcworkspace (111.08 KB, patch)
2020-09-22 19:08 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Part 1 (for EWS) (191.20 KB, patch)
2020-09-23 08:51 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Part 1: Enable parallel schemes in WebKit.xcworkspace (191.09 KB, patch)
2020-09-23 16:04 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Part 1: Enable parallel schemes in WebKit.xcworkspace (212.97 KB, patch)
2020-09-24 17:44 PDT, Andy Estes
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2017-10-12 12:24:49 PDT
We should figure out why we can't do this now and what we need to change to fix this. This hasn't been attempted in a long time so there's a lot of unknowns here. Also, since the command line build doesn't use schemes we should see if you can parallelize the build there too.
Comment 1 Radar WebKit Bug Importer 2017-10-12 12:25:08 PDT
<rdar://problem/34960577>
Comment 2 Andy Estes 2020-09-22 07:50:21 PDT
I was able to improve the full build time in a workspace by 20% with this option enabled. It required some project changes that I'll post a patch for here.

It's also possible to parallelize the targets within each project when building from the command line with the "Parallelize build for command-line builds" project option, but that's not as fast as allowing full target parallelization across a workspace of projects.

We could make the command line build faster by invoking a different xcodebuild command, like `xcodebuild -workspace WebKit.workspace -scheme "All Tools"`, when make is run from a directory containing a workspace. Building the "All Tools" scheme with "Find Implicit Dependencies" enabled is equivalent to what `make debug` from the root directory, for instance.
Comment 3 Keith Miller 2020-09-22 11:47:55 PDT
(In reply to Andy Estes from comment #2)
> We could make the command line build faster by invoking a different
> xcodebuild command, like `xcodebuild -workspace WebKit.workspace -scheme
> "All Tools"`, when make is run from a directory containing a workspace.
> Building the "All Tools" scheme with "Find Implicit Dependencies" enabled is
> equivalent to what `make debug` from the root directory, for instance.

Does building with a full workspace cause TestWTF to be built very early in the build? Whenever I've built with our internal build I noticed that it built right away. I'd be nice (at least for jsc devs) if that was done after the rest of the JSC build.

That said, maybe we should just have a JSC CLI build target... 🤷‍♂️
Comment 4 mitz 2020-09-22 12:03:49 PDT
(In reply to Keith Miller from comment #3)
>
> Does building with a full workspace cause TestWTF to be built very early in
> the build? Whenever I've built with our internal build I noticed that it
> built right away. I'd be nice (at least for jsc devs) if that was done after
> the rest of the JSC build.

Is that desirable for efficiency or is there actually a TestWTF dependency on JSC that Xcode doesn’t know about?
Comment 5 Keith Miller 2020-09-22 12:05:24 PDT
(In reply to mitz from comment #4)
> (In reply to Keith Miller from comment #3)
> >
> > Does building with a full workspace cause TestWTF to be built very early in
> > the build? Whenever I've built with our internal build I noticed that it
> > built right away. I'd be nice (at least for jsc devs) if that was done after
> > the rest of the JSC build.
> 
> Is that desirable for efficiency or is there actually a TestWTF dependency
> on JSC that Xcode doesn’t know about?

No, just efficiency. I don't know of any true dependency. FWIW, if you're trying to test WTR you also don
Comment 6 Keith Miller 2020-09-22 12:06:20 PDT
(In reply to Keith Miller from comment #5)
> (In reply to mitz from comment #4)
> > (In reply to Keith Miller from comment #3)
> > >
> > > Does building with a full workspace cause TestWTF to be built very early in
> > > the build? Whenever I've built with our internal build I noticed that it
> > > built right away. I'd be nice (at least for jsc devs) if that was done after
> > > the rest of the JSC build.
> > 
> > Is that desirable for efficiency or is there actually a TestWTF dependency
> > on JSC that Xcode doesn’t know about?
> 
> No, just efficiency. I don't know of any true dependency. FWIW, if you're
> trying to test WTR you also don

Ugh, Tab+enter in the text field is a bad combo... lol

don't want to burn CPU cycles building TestWTF first.
Comment 7 Andy Estes 2020-09-22 13:22:31 PDT
(In reply to Keith Miller from comment #3)
> (In reply to Andy Estes from comment #2)
> > We could make the command line build faster by invoking a different
> > xcodebuild command, like `xcodebuild -workspace WebKit.workspace -scheme
> > "All Tools"`, when make is run from a directory containing a workspace.
> > Building the "All Tools" scheme with "Find Implicit Dependencies" enabled is
> > equivalent to what `make debug` from the root directory, for instance.
> 
> Does building with a full workspace cause TestWTF to be built very early in
> the build? Whenever I've built with our internal build I noticed that it
> built right away. I'd be nice (at least for jsc devs) if that was done after
> the rest of the JSC build.
> 
> That said, maybe we should just have a JSC CLI build target... 🤷‍♂️

There isn't a way to build the full workspace, per se. You create a scheme that lists the targets you want to build, and Xcode builds just those targets and their dependencies (incl. dependent targets from other workspace projects when "Find Implicit Dependencies" is enabled).

Existing WebKit.xcworkspace schemes like "All Tools" might build both TestWebKitAPI and TestWTF, but you can always create your own scheme that builds just the targets you want.
Comment 8 Andy Estes 2020-09-22 13:32:57 PDT
(In reply to mitz from comment #4)
> (In reply to Keith Miller from comment #3)
> >
> > Does building with a full workspace cause TestWTF to be built very early in
> > the build? Whenever I've built with our internal build I noticed that it
> > built right away. I'd be nice (at least for jsc devs) if that was done after
> > the rest of the JSC build.
> 
> Is that desirable for efficiency or is there actually a TestWTF dependency
> on JSC that Xcode doesn’t know about?

Despite its name, for the purposes of parallelizing targets TestWTF depends on WebKit just like TestWebKitAPI does, because both share the same config.h that includes WebKit headers (although that could be fixed).
Comment 9 Andy Estes 2020-09-22 17:06:29 PDT Comment hidden (obsolete)
Comment 10 Andy Estes 2020-09-22 18:50:34 PDT Comment hidden (obsolete)
Comment 11 Andy Estes 2020-09-22 18:58:39 PDT Comment hidden (obsolete)
Comment 12 Andy Estes 2020-09-22 19:08:27 PDT Comment hidden (obsolete)
Comment 13 mitz 2020-09-22 19:16:45 PDT
Comment on attachment 409439 [details]
Enable "Parallelize Build" option in WebKit.xcworkspace

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

> WebKit.xcworkspace/contents.xcworkspacedata:50
> +      location = "group:../Internal/WebKit/WebKitAdditions/WebKitAdditions.xcodeproj">

Is this going to create any problems for folks who don’t have Internal?
Comment 14 Andy Estes 2020-09-22 19:21:08 PDT
Comment on attachment 409439 [details]
Enable "Parallelize Build" option in WebKit.xcworkspace

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

> Source/JavaScriptCore/ChangeLog:25
> +        dependent target's products from the build directory. If this build phase has the

s/dependent/depended-on/
Comment 15 Andy Estes 2020-09-22 19:22:10 PDT
(In reply to mitz from comment #13)
> Comment on attachment 409439 [details]
> Enable "Parallelize Build" option in WebKit.xcworkspace
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=409439&action=review
> 
> > WebKit.xcworkspace/contents.xcworkspacedata:50
> > +      location = "group:../Internal/WebKit/WebKitAdditions/WebKitAdditions.xcodeproj">
> 
> Is this going to create any problems for folks who don’t have Internal?

My expectation is no, but I need to test.
Comment 16 mitz 2020-09-22 19:25:18 PDT
Comment on attachment 409439 [details]
Enable "Parallelize Build" option in WebKit.xcworkspace

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

> Source/JavaScriptCore/ChangeLog:26
> +        "Run script only when installing" option enabled and exists in a target with SKIP_INSTALL=YES

“Copy only when installing”
Comment 17 mitz 2020-09-22 19:33:14 PDT
Comment on attachment 409439 [details]
Enable "Parallelize Build" option in WebKit.xcworkspace

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

> Source/JavaScriptCore/ChangeLog:29
> +

It looks like in production builds (when invoked with `xcodebuild install` and therefore DEPLOYMENT_LOCATION is `YES`), Xcode will still perform a copy into somewhere in OBJROOT or some temporary location, since SKIP_INSTALL doesn’t mean that the target doesn’t get built, only that it doesn’t get put directly into DSTROOT. I didn’t test this extensively, but if you’re seeing the same effect and you think it’s undesirable, one way that I’ve found to cancel it out is to exclude the file using EXCLUDED_SOURCE_FILE_NAMES. Implicit dependency tracking doesn’t check that, but copy phase processing does.
Comment 18 Andy Estes 2020-09-22 19:36:35 PDT
(In reply to Andy Estes from comment #15)
> (In reply to mitz from comment #13)
> > Comment on attachment 409439 [details]
> > Enable "Parallelize Build" option in WebKit.xcworkspace
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=409439&action=review
> > 
> > > WebKit.xcworkspace/contents.xcworkspacedata:50
> > > +      location = "group:../Internal/WebKit/WebKitAdditions/WebKitAdditions.xcodeproj">
> > 
> > Is this going to create any problems for folks who don’t have Internal?
> 
> My expectation is no, but I need to test.

I don't think this creates problems.

Xcode ignores missing projects when opening a workspace, and none of this project's targets are in any of the shared schemes. When Copy WTF Headers builds, it fails to find a project that can build the libWebKitAdditionsPlaceholder.a product, but otherwise continues to treat the Build Workspace Products phase as just a no-op.
Comment 19 Andy Estes 2020-09-22 20:47:39 PDT
(In reply to Andy Estes from comment #18)
> I don't think this creates problems.

(In reply to mitz from comment #17)
> Xcode will still perform a copy into somewhere in OBJROOT or some temporary
> location, since SKIP_INSTALL doesn’t mean that the target doesn’t get built,
> only that it doesn’t get put directly into DSTROOT.

In that case, non-existent workspace products create problems :)

A Product > Archive build in Xcode seems to show one of them. Surprisingly, the tool I used to build this patch for Production did not fail, or even show the "Build Workspace Products" phase running (in the places I checked for it).

Thank you for pointing this out. I'll post a new patch that uses EXCLUDED_SOURCE_FILE_NAMES for these products.
Comment 20 Keith Rollin 2020-09-22 22:50:59 PDT
Please be sure to test with XCBuild. XCBuild is all about the dependencies, and since you're fiddling with those, it's a good idea to make sure you don't confound it. We've seen issues along these lines before.

To build with XCBuild:

* With `make`: Add `ARGS=-UseNewBuildSystem=YES` to the command line.
* With `build-webkit`: Add --xcbuild to the command-line.
* With production build scripts: Add --xcbuild to the command line.
* With Xcode: Make sure your patch is safely in git. Then set File > WorkSpace Settings... > Shared Workspace Settings > Build System = New Build System. This will change your workspace and projects in your working directory. By keeping these changes separate from your patch, you can easily revert them to their pre-XCBuild state so that you can submit just your changes.
Comment 21 Andy Estes 2020-09-22 23:01:39 PDT
(In reply to Keith Rollin from comment #20)
> Please be sure to test with XCBuild.

I’ve successfully built this patch from both the workspace and make(1) using both the legacy and new build systems.
Comment 22 Keith Rollin 2020-09-22 23:09:11 PDT
(In reply to Andy Estes from comment #21)
> (In reply to Keith Rollin from comment #20)
> > Please be sure to test with XCBuild.
> 
> I’ve successfully built this patch from both the workspace and make(1) using
> both the legacy and new build systems.

Awesome. You're a god.
Comment 23 Andy Estes 2020-09-23 08:51:27 PDT Comment hidden (obsolete)
Comment 24 Saam Barati 2020-09-23 11:24:30 PDT
When this lands, will running `make release` just give me this faster build?
Comment 25 Andy Estes 2020-09-23 16:04:36 PDT Comment hidden (obsolete)
Comment 26 Andy Estes 2020-09-23 16:08:29 PDT
(In reply to Saam Barati from comment #24)
> When this lands, will running `make release` just give me this faster build?

Not in this first part, but I plan to post another patch that parallelizes `make` builds before I resolve this bug.
Comment 27 Andy Estes 2020-09-23 16:41:17 PDT
Here is my plan for the `make` build:

1. The top-level Makefile will invoke `xcodebuild` once to build a parallel WebKit.xcworkspace scheme that builds everything. That build will be about as fast on the command line as it is in the Xcode UI.

2. Invoking make from other directories will continue to recursively build individual projects, but limited parallelism can be achieved by enabling "Parallelize build for command-line builds" in supported projects. Top-level workspace builds followed by recursive incremental rebuilds in sub-directories will be supported.

2a. I also considered having each project directory have a scheme in WebKit.xcworkspace that is built instead of the project itself, e.g., `make -C Source/JavaScriptCore` would rebuild workspace dependencies automatically before building JavaScriptCore. I'm worried this change would disrupt some workflows, though.
Comment 28 Andy Estes 2020-09-23 16:44:31 PDT
2b. The Source/ and Tools/ top-level sub-directories are also candidates to build specially-crafted parallel workspace schemes rather than doing recursive `make`.
Comment 29 Andy Estes 2020-09-24 17:44:38 PDT
Created attachment 409641 [details]
Part 1: Enable parallel schemes in WebKit.xcworkspace
Comment 30 Andy Estes 2020-09-24 17:46:16 PDT
(In reply to Andy Estes from comment #29)
> Created attachment 409641 [details]
> Part 1: Enable parallel schemes in WebKit.xcworkspace

- Fixed an issue with building libwebrtc in parallel on the new build system.
- Fixed an issue where some JSC framework symlinks weren't created when building in parallel.
Comment 31 Darin Adler 2020-09-26 16:17:13 PDT
Comment on attachment 409641 [details]
Part 1: Enable parallel schemes in WebKit.xcworkspace

I suppose the main issue here is that when you specify inputs and outputs we have to be sure both lists are exhaustive. I wasn’t able to audit each list. This is a welcome improvement, but will cause subtle failures if we left out any input or output files.

For example, in steps that execute scripts, we want to list the scripts themselves *plus anything they invoke, including perl modules as well as perl scripts and all the python sources*. I’m guessing you did all that carefully, but I am not sure!
Comment 32 Elliott Williams 2022-06-17 17:57:22 PDT
This has been done in WebKit.xcworkspace since <https://bugs.webkit.org/show_bug.cgi?id=235385>, as part of our migration to XCBuild. build-webkit uses the parallel workspace build by default as of this week <https://bugs.webkit.org/show_bug.cgi?id=241298>.