Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support true parallel builds #225

Closed
wants to merge 10 commits into from
Closed

Support true parallel builds #225

wants to merge 10 commits into from

Conversation

kirb
Copy link
Member

@kirb kirb commented Jan 28, 2017

What does this implement/fix? Explain your changes.

Enables true parallel builds, either when specifying the -j flag, and by default using as many logical cores (e.g. 8 on a Core i7, 4 on a Core i5, 2 on an iPhone 6s). We just need to pass $(MFLAGS) to sub-make calls, so that the job server flag is included (so sub-makes can communicate with the parent and each other, ensuring they only use the specified number of threads at a time). Without the flag, sub-makes revert to working serially:

make[4]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.

Does this close any currently open issues?

No

Any other comments?

  • Usually make is meant to inject the jobserver flags when it sees $(MAKE) being used, but that wasn’t working for me, even when adding a + at the start of the line as the warning suggests. However, there is $(MFLAGS), which is ”for historical compatibility,” but does exactly what we want. I was Doing It Wrong™. $(MAKEFLAGS) is a set of flags implicitly passed down to sub-makes via the environment; it doesn’t have to be passed in manually.

  • This also adds --output-sync=target to the MAKEFLAGS, so the output doesn't look like vomit if there are errors (although it sure makes it look less fast). This isn't supported by (you guessed it) Apple's ancient version of make. So for now, this requires macOS users to: As such parallelism isn’t supported on macOS unless make is updated to the latest via:

    $ brew install make
    $ alias make=gmake
  • At the moment there are still some cases where it might output: Fixed.

    make[4]: warning: -jN forced in submake: disabling jobserver mode.
    
  • I want to make parallelism completely implicit if at all possible. As long as the user hasn’t passed a -j flag of their own on the command line, Theos should automatically determine the appropriate number of jobs to run at once, and then re-run make with the same command line plus, eg, -j 4 for a 4-thread CPU. Done.

Where has this been tested?

  • Operating System: macOS 10.12.3
  • Target Platform: iOS
  • Toolchain Version: Xcode 8.1
  • SDK Version: iOS 10.1

This also adds --output-sync=target to the MFLAGS, so the output doesn't look
like vomit if there are errors (although it sure makes it look less fast). This
isn't supported by (you guessed it) Apple's ancient version of make. So for now,
this requires macOS users to `brew install make` and `alias make=gmake`.
@kirb kirb self-assigned this Jan 28, 2017
@kirb kirb changed the title Support parallel builds Support true parallel builds Jan 28, 2017
@kirb
Copy link
Member Author

kirb commented Dec 9, 2017

I’m confident in this now. I haven’t been able to get it to complain about passing -j at the same time as --jobserver-auth (I check for that flag in $(MAKEFLAGS), which seems to be what the make manual recommends), and it now determines a reasonable default number of threads (equal to number of logical cores on your CPU) to pass to second-level make invocations. It also checks for Apple’s super old make and disables parallelism, and can be disabled with THEOS_USE_PARALLEL_BUILDING = no. I guess it can be merged 😛

@kirb kirb requested review from uroboro and removed request for uroboro December 9, 2017 08:54
When using --output-sync, build commands are given a normal file descriptor as
their stdout/stderr, rather than the stdout/stderr of the tty. This means they
don’t see a tty when they do `isatty(stderr)` and therefore don’t give colored
output or any other fancy stuff. This forces that by using -fcolor-diganostics.
@kirb kirb closed this Jan 25, 2018
@kirb kirb deleted the kirb/implement/parallel branch January 25, 2018 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants