Closed Bug 915537 Opened 11 years ago Closed 11 years ago

mach build --jobs doesn't work.

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla28

People

(Reporter: glandium, Assigned: glandium)

Details

Attachments

(1 file, 1 obsolete file)

When running mach build --jobs N, make is still run with -jncpu, not -jN.
Component: mach → Build Config
Might be because of the allow_parallel=False bit, which was there already in bug 867966. Apparently -j never worked for full-tree builds?
I hit this problem when building B2G; more precisely I noticed that no matter how many jobs I gave './mach build' it would always run one job per CPU core while at the same time specifying a number of jobs in .mozconfig worked correctly.

After a bit of fiddling within the code I noticed that MozbuildObject._run_make() is always invoked with the |allow_parallel| parameter set to false, see here:

http://hg.mozilla.org/mozilla-central/file/bc8c1eb0f2ba/python/mozbuild/mozbuild/mach_commands.py#l386
http://hg.mozilla.org/mozilla-central/file/bc8c1eb0f2ba/python/mozbuild/mozbuild/mach_commands.py#l453

This in turns causes this bit of code not to add any '-j' parameter to the make invocation:

http://hg.mozilla.org/mozilla-central/file/bc8c1eb0f2ba/python/mozbuild/mozbuild/base.py#l412

Which in turn causes the makefile to either pick the number of jobs from the .mozconfig file or to compute them on his own:

http://hg.mozilla.org/mozilla-central/file/bc8c1eb0f2ba/client.mk#l132
Yes, indeed. Want to fix it?
(In reply to :Ms2ger from comment #3)
> Yes, indeed. Want to fix it?

Yup, I've got a working patch ready :)
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Here's a patch that fixes the problem, it turned out to be a little bit more complicated than I thought:

 |MozbuildObject._run_make()| was always invoked with |allow_parallel| set to false so it never injected the |-j| option in the make arguments. However even having fixed that part client.mk would still override this value because it checked for the |-j| option within the |MOZ_MAKE_FLAGS| variable before providing its own number of jobs. The parameters passed to the make invocation by mach were not stored there, so in the end client.mk would always specify it's own value for |-j| ignoring whatever mach had provided.

 This also caused another problem: when |MozbuildObject._run_make()| was invoked with |allow_parallel| set to false it would still run a parallel job as the code in client.mk would override the number of jobs anyway.

The patch I've attached fixes both problems by passing the number of jobs in the |MOZ_MAKE_FLAGS| variable besides the |-j| parameter from within mach and also enforces a single job when |allow_parallel| is false. I'm not sure if this is the right approach but it works.
Attachment #830126 - Flags: review?(mh+mozilla)
client.mk is *always* invoked with -j1 because that's how client.mk rolls.

I suppose you could pass a MOZ_MAKE_FLAGS environment variable into client.mk. Although if your mozconfig redefines that variable then it will get overwritten. I'd probably r- a trivial patch to do things this way because it introduces lots of undefined and unexpected behavior.

I would like to see us kill mk_add_options b/c it's effectively duplicated by mach arguments. But the automation mozconfigs use mk_add_options, so it's not a trivial patch. I suppose we could not support mk_add_options if building with mach. But that's also not a trivial patch.

An even better (but more involved) solution is to rewrite the client.mk logic in Python. That has been on our long todo list. But we can't justify working on it while we still have major build speed projects on the plate.

tl;dr the reason this is still a bug is because the fix isn't trivial.
(In reply to Gregory Szorc [:gps] from comment #6)
> I would like to see us kill mk_add_options b/c it's effectively duplicated
> by mach arguments.

That's simply not true. There are only a few mach arguments, compared to what can be done with mk_add_options. Automation mozconfigs are an obvious example of that.

Disabling mk_add_options when building with mach would also likely break things unexpectedly. Because mk_add_options is not for MOZ_MAKE_FLAGS=-jN only.

That being said, I think a solution here would be one that does the following:
- running mach build would build with the -jN value from MOZ_MAKE_FLAGS, or, if there aren't any, use the number of cpus. Guess what, that's what we already do!
- running mach build -jN would always make client.mk use that value, whatever it has in MOZ_MAKE_FLAGS. But that shouldn't replace other things that would be in MOZ_MAKE_FLAGS.
Attachment #830126 - Flags: review?(mh+mozilla)
Attachment #830552 - Flags: review?(gps)
Assignee: gsvelto → mh+mozilla
Attachment #830126 - Attachment is obsolete: true
Attachment #830552 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/10fecade0b53
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.