Jobserver fifo#2
Closed
hundeboll wants to merge 18 commits intoKitware:kitware-staged-featuresfrom
Closed
Conversation
This will fix the issue (ninja-build#2173).
- add new TokenPool interface - GNU make implementation for TokenPool parses and verifies the magic information from the MAKEFLAGS environment variable - RealCommandRunner tries to acquire TokenPool * if no token pool is available then there is no change in behaviour - When a token pool is available then RealCommandRunner behaviour changes as follows * CanRunMore() only returns true if TokenPool::Acquire() returns true * StartCommand() calls TokenPool::Reserve() * WaitForCommand() calls TokenPool::Release() Documentation for GNU make jobserver http://make.mad-scientist.net/papers/jobserver-implementation/ Fixes ninja-build#1139
Improve on the original jobserver client implementation. This makes ninja a more aggressive GNU make jobserver client. - add monitor interface to TokenPool - TokenPool is passed down when main loop indicates that more work is ready and would be allowed to start if a token becomes available - posix: update DoWork() to monitor TokenPool read file descriptor - WaitForCommand() exits when DoWork() sets token flag - Main loop starts over when WaitForCommand() sets token exit status
This emulates the behaviour of GNU make. - add parallelism_from_cmdline flag to build configuration - set the flag when -jN is given on command line - pass the flag to TokenPool::Get() - GNUmakeTokenPool::Setup() * prints a warning when the flag is true and jobserver was detected * returns false, i.e. jobserver will be ignored - ignore config.parallelism in CanRunMore() when we have a valid TokenPool, because it gets always initialized to a default when not given on the command line
This emulates the behaviour of GNU make. - build: make a copy of max_load_average and pass it to TokenPool. - GNUmakeTokenPool: if we detect a jobserver and a valid -lN argument in MAKEFLAGS then set max_load_average to N.
- replace printf() with calls to LinePrinter - print GNU make jobserver message only when verbose build is requested
- fix Windows build error in no-op TokenPool implementation - improve Acquire() to block for a maximum of 100ms - address review comments
- TokenPool setup - GetMonitorFd() API - implicit token and tokens in jobserver pipe - Acquire() / Reserve() / Release() protocol - Clear() API
- add TokenPoolTest stub to provide TokenPool::GetMonitorFd() - add two tests * both tests set up a dummy GNUmake jobserver pipe * both tests call DoWork() with TokenPoolTest * test 1: verify that DoWork() detects when a token is available * test 2: verify that DoWork() works as before without a token - the tests are not compiled in under Windows
Add tests that verify the token functionality of the builder main loop. We replace the default fake command runner with a special version where the tests can control each call to AcquireToken(), CanRunMore() and WaitForCommand().
GNU make uses a semaphore as jobserver protocol on Win32. See also https://www.gnu.org/software/make/manual/html_node/Windows-Jobserver.html Usage is pretty simple and straightforward, i.e. WaitForSingleObject() to obtain a token and ReleaseSemaphore() to return it. Unfortunately subprocess-win32.cc uses an I/O completion port (IOCP). IOCPs aren't waitable objects, i.e. we can't use WaitForMultipleObjects() to wait on the IOCP and the token semaphore at the same time. Therefore GNUmakeTokenPoolWin32 creates a child thread that waits on the token semaphore and posts a dummy I/O completion status on the IOCP when it was able to obtain a token. That unblocks SubprocessSet::DoWork() and it can then check if a token became available or not. - split existing GNUmakeTokenPool into common and platform bits - add GNUmakeTokenPool interface - move the Posix bits to GNUmakeTokenPoolPosix - add the Win32 bits as GNUmakeTokenPoolWin32 - move Setup() method up to TokenPool interface - update Subprocess & TokenPool tests accordingly
- remove unnecessary "struct" from TokenPool - add PAPCFUNC cast to QueryUserAPC() - remove hard-coded MAKEFLAGS string from win32 - remove useless build test CompleteNoWork - rename TokenPoolTest to TestTokenPool - add tokenpool modules to CMake build - remove unused no-op TokenPool implementation - address review comments from ninja-build#1140 (review) ninja-build#1140 (review) ninja-build#1140 (comment) ninja-build#1140 (comment)
Pending upstream integration of the features added by this branch, extend the version number of Ninja to indicate that it has the extra features so that tools can detect availability.
Make space to add new API to set up token pool master.
GNU make 4.4 introduced a new jobserver style "fifo" for POSIX systems which passes a named pipe down to the clients. - update auth parser to recognize "fifo:<name>" format - open named pipe for reading and writing - make sure the file descriptors are closed in the destructor - add 2 tests that aren't compiled for WIN32
Author
|
@bradking Can you take a look on this, or ping someone who can? |
Member
|
@hundeboll thanks. In general we don't accept pull requests directly to this fork. It exists only to stage upstream PRs that haven't been merged there yet. This PR is toward that goal, but I'd like to avoid cherry-picking if possible. When I get a chance I'll look through the current state of the relevant upstream PRs to see if I can rebuild our fork. |
bradking
force-pushed
the
kitware-staged-features
branch
2 times, most recently
from
e64cee3 to
3501c48
Compare
bradking
force-pushed
the
kitware-staged-features
branch
from
3501c48 to
4aa57b7
Compare
bradking
force-pushed
the
kitware-staged-features
branch
from
4aa57b7 to
d74efef
Compare
Member
|
Ninja 1.13.0 has been released with jobserver fifo support. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recent GNU make versions support opening a fifo path instead of using passed filedescriptor numbers. This simplifies using the jobserver functionality in build systems like openembedded.
These two patches are cherry-picked from ninja-build#2263 while skipping the patches adding joberserver master support.
A few simple conflicts were resolved in the test file while cherry-picking.