Page MenuHomePhabricator

[DEPENDS] Use parallel compilation when building packages
ClosedPublic

Authored by Fabien on Jan 8 2020, 09:01.

Details

Summary

This diffs try to guess the number of cores availble on the user machine
and forward it (+1) to the -jX option to make when building the
packages.
This JOBS variable can be overriden by the user and fallback to 1 if
it can't guess the number of cores.

For now only ccache makes use of this feature. This is built on all the platforms and is simple enough for demonstrating the feature.

Test Plan
cd depends

Edit packages/native_ccache.mk, add the line:

echo "BUILDING WITH -j$(JOBS)" && \

before the line:

$(MAKE) -j$(JOBS)

make V=1

Check in the output that make is invoked with the appropriated -jX
option(for the ccache package).

make V=1 JOBS=2

Check in the output that make is invoked with the appropriated -j2
option (for the ccache package).

make V=1 build-all

Check in the output that the JOBS is propagated to the ccache package of all the targets.

Run the Gitian builds and check the output is still deterministic (clear
the cached packages before building).

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Jan 8 2020, 09:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 8 2020, 09:01

Can you make sure that JOB is respected properly when using build-all ?

depends/Makefile
20 ↗(On Diff #15217)

Make it zero at the end and always add 1 so we get n+1 thread, which is usually considered what's good.

deadalnix requested changes to this revision.Jan 8 2020, 13:20

I'd make sure to get the mechanism working using one of the package before extending it to all packages. This is hard to review and any change to one package is going to block everything.

depends/packages/boost.mk
36 ↗(On Diff #15217)

Why is it removed?

40 ↗(On Diff #15217)

Why -j4 ?

This revision now requires changes to proceed.Jan 8 2020, 13:20
Fabien added a comment.Jan 8 2020, 13:31

OK, I will make it on a single package to begin with.

depends/packages/boost.mk
36 ↗(On Diff #15217)
40 ↗(On Diff #15217)

I have no idea where this limit comes from.
I did not change the install commands since there is not much to gain here (I tested, the difference is not significant).

Fabien planned changes to this revision.Jan 8 2020, 13:31
Fabien edited the summary of this revision. (Show Details)Jan 8 2020, 13:47
Fabien edited the test plan for this revision. (Show Details)
Fabien updated this revision to Diff 15219.Jan 8 2020, 13:51

Address feedback:

  • Use N+1 jobs, where N is the number of cores
  • Only use JOBS in a single package (ccache)
  • Add a build-all check to the test plan
Fabien added a comment.Jan 8 2020, 13:52

Note to reviewers:
The build-all check from the test plan is still running on my machine (so far so good).
I will put a notice in a later comment when it ends.

Note to reviewers:
The build-all check from the test plan is still running on my machine (so far so good).
I will put a notice in a later comment when it ends.

You don't need check for the build to terminate, you need to check for the right value of JOBS to be passed down.

deadalnix accepted this revision.Jan 8 2020, 13:57
This revision is now accepted and ready to land.Jan 8 2020, 13:57
This revision was automatically updated to reflect the committed changes.