-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement GNU Make 4.4+ jobserver fifo / semaphore client support #2450
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,7 @@ | |
#include "depfile_parser.h" | ||
#include "exit_status.h" | ||
#include "graph.h" | ||
#include "jobserver.h" | ||
#include "util.h" // int64_t | ||
|
||
struct BuildLog; | ||
|
@@ -52,6 +53,9 @@ struct Plan { | |
/// Returns true if there's more work to be done. | ||
bool more_to_do() const { return wanted_edges_ > 0 && command_edges_ > 0; } | ||
|
||
/// Jobserver status used to skip capacity based on load average | ||
bool JobserverEnabled() const; | ||
|
||
/// Dumps the current state of the plan. | ||
void Dump() const; | ||
|
||
|
@@ -139,14 +143,17 @@ struct Plan { | |
|
||
/// Total remaining number of wanted edges. | ||
int wanted_edges_; | ||
|
||
/// Jobserver client | ||
Jobserver jobserver_; | ||
}; | ||
|
||
/// CommandRunner is an interface that wraps running the build | ||
/// subcommands. This allows tests to abstract out running commands. | ||
/// RealCommandRunner is an implementation that actually runs commands. | ||
struct CommandRunner { | ||
virtual ~CommandRunner() {} | ||
virtual size_t CanRunMore() const = 0; | ||
virtual size_t CanRunMore(bool jobserver_enabled) const = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest not changing this method's signature at all, to minimize changes. Since the flag value is not going to change during a build, you can simply pass This should work identically, without having to change the DryCommandRunner and TestCommandRunner interfaces / implementations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. |
||
virtual bool StartCommand(Edge* edge) = 0; | ||
|
||
/// The result of waiting for a command. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// Copyright 2024 Google Inc. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#include "jobserver.h" | ||
|
||
#include <fcntl.h> | ||
#include <unistd.h> | ||
|
||
#include <cassert> | ||
#include <cstring> | ||
|
||
#include "util.h" | ||
|
||
void Jobserver::Init() { | ||
assert(fd_ < 0); | ||
|
||
if (!ParseJobserverAuth("fifo")) { | ||
return; | ||
} | ||
|
||
const char* jobserver = jobserver_name_.c_str(); | ||
|
||
fd_ = open(jobserver, O_NONBLOCK | O_CLOEXEC | O_RDWR); | ||
if (fd_ < 0) { | ||
Fatal("failed to open jobserver: %s: %s", jobserver, strerror(errno)); | ||
} | ||
|
||
Info("using jobserver: %s", jobserver); | ||
} | ||
|
||
Jobserver::~Jobserver() { | ||
assert(token_count_ == 0); | ||
|
||
if (fd_ >= 0) { | ||
close(fd_); | ||
} | ||
} | ||
|
||
bool Jobserver::Enabled() const { | ||
return fd_ >= 0; | ||
} | ||
|
||
bool Jobserver::AcquireToken() { | ||
char token; | ||
int res = read(fd_, &token, 1); | ||
hundeboll marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (res < 0 && errno != EAGAIN && errno != EWOULDBLOCK) { | ||
Fatal("failed to read jobserver token: %s", strerror(errno)); | ||
} | ||
|
||
return res > 0; | ||
} | ||
|
||
void Jobserver::ReleaseToken() { | ||
char token = '+'; | ||
Comment on lines
+64
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from the Make manual:
I have not looked at Make's code relevant to this yet, so I don't know the significance. It might be that the character being anything other than a I will follow up with another comment with more information as I continue to play with the PR's changes and review Make's source. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I've noticed the same from the Make manual. It might make sense to store the acquired token in the (recently added) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After looking at GNU Make source history, I changed my mind. I don't think this is important. There is no sign that they are going to introduce new characters that have special meaning, and if they somehow do that, it would be pretty easy to add support for it later. Let's save the idea for the future... |
||
int res = write(fd_, &token, 1); | ||
hundeboll marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (res != 1) { | ||
Fatal("failed to write token: %s: %s", jobserver_name_.c_str(), | ||
strerror(errno)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
// Copyright 2024 Google Inc. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
#include "jobserver.h" | ||
|
||
#include <windows.h> | ||
|
||
#include <cassert> | ||
|
||
#include "util.h" | ||
|
||
void Jobserver::Init() { | ||
assert(sem_ == INVALID_HANDLE_VALUE); | ||
|
||
if (!ParseJobserverAuth("sem")) { | ||
return; | ||
} | ||
|
||
const char* name = jobserver_name_.c_str(); | ||
|
||
sem_ = OpenSemaphore(SYNCHRONIZE|SEMAPHORE_MODIFY_STATE, false, name); | ||
if (sem_ == nullptr) { | ||
Win32Fatal("OpenSemaphore", name); | ||
} | ||
|
||
Info("using jobserver: %s", name); | ||
hundeboll marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
Jobserver::~Jobserver() { | ||
assert(token_count_ == 0); | ||
|
||
if (sem_ != INVALID_HANDLE_VALUE) { | ||
CloseHandle(sem_); | ||
} | ||
} | ||
|
||
bool Jobserver::Enabled() const { | ||
return sem_ != INVALID_HANDLE_VALUE; | ||
} | ||
|
||
bool Jobserver::AcquireToken() { | ||
return WaitForSingleObject(sem_, 0) == WAIT_OBJECT_0; | ||
} | ||
|
||
void Jobserver::ReleaseToken() { | ||
if (!ReleaseSemaphore(sem_, 1, nullptr)) { | ||
Win32Fatal("ReleaseSemaphore"); | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.