Skip to content

Commit d03e75f

Browse files
committed
build-cores: detect cores automatically if set to 0
This changes makes nix detect a machines available cores automatically whenever build-cores is set to 0. So far, nix simply passed NIX_BUILD_CORES=0 whenever build-cores is set to 0. (only when build-cores is unset it was detecting cores automatically) The behavior of passing NIX_BUILD_CORES=0 leads to a performance penalty when sourcing nixpkgs' generic builder's `setup.sh`, as setup.sh has to execute `nproc`. This significantly slows down sourcing of setup.sh This affects all nixos machines since the introduction of the nix settings module a couple releases ago which sets build-cores to 0 be default.
1 parent df2d5f2 commit d03e75f

File tree

7 files changed

+151
-8
lines changed

7 files changed

+151
-8
lines changed

src/libstore-tests/globals.cc

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#include <gtest/gtest.h>
2+
3+
#include "nix/store/globals.hh"
4+
5+
namespace nix {
6+
7+
/**
8+
* Test the BuildCoresSetting to ensure build-cores=0 gets resolved to
9+
* getDefaultCores() at configuration time.
10+
*/
11+
class BuildCoresSettingTest : public ::testing::Test {
12+
protected:
13+
void SetUp() override {
14+
// Save the original buildCores setting to restore after each test
15+
originalBuildCores = settings.buildCores;
16+
}
17+
18+
void TearDown() override {
19+
// Restore the original buildCores setting
20+
settings.buildCores = originalBuildCores;
21+
}
22+
23+
unsigned int originalBuildCores;
24+
};
25+
26+
TEST_F(BuildCoresSettingTest, buildCores_withNonZeroValue) {
27+
// When buildCores is set to a non-zero value, it should store that value
28+
settings.buildCores.set("4");
29+
EXPECT_EQ(settings.buildCores, 4U);
30+
31+
settings.buildCores.set("1");
32+
EXPECT_EQ(settings.buildCores, 1U);
33+
}
34+
35+
TEST_F(BuildCoresSettingTest, buildCores_withZeroString) {
36+
// When buildCores is set to "0", it should resolve to getDefaultCores() at config time
37+
unsigned int defaultCores = settings.getDefaultCores();
38+
39+
settings.buildCores.set("0");
40+
41+
// After setting to "0", buildCores should contain the default cores value, not 0
42+
EXPECT_EQ(settings.buildCores, defaultCores);
43+
EXPECT_NE(settings.buildCores, 0U);
44+
// The resolution of buildCores=0 should always be positive
45+
EXPECT_GT(settings.buildCores, 0U);
46+
}
47+
48+
TEST_F(BuildCoresSettingTest, buildCores_invalidStringThrows) {
49+
// Invalid strings should throw an error
50+
EXPECT_THROW(settings.buildCores.set("invalid"), UsageError);
51+
EXPECT_THROW(settings.buildCores.set(""), UsageError);
52+
EXPECT_THROW(settings.buildCores.set("auto"), UsageError); // auto is not supported for buildCores
53+
}
54+
55+
} // namespace nix

src/libstore-tests/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ sources = files(
5959
'derivation.cc',
6060
'derived-path.cc',
6161
'downstream-placeholder.cc',
62+
'globals.cc',
6263
'http-binary-cache-store.cc',
6364
'legacy-ssh-store.cc',
6465
'local-binary-cache-store.cc',

src/libstore/globals.cc

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ std::vector<Path> getUserConfigFiles()
140140
return files;
141141
}
142142

143-
unsigned int Settings::getDefaultCores()
143+
unsigned int Settings::getDefaultCores() const
144144
{
145145
const unsigned int concurrency = std::max(1U, std::thread::hardware_concurrency());
146146
const unsigned int maxCPU = getMaxCPU();
@@ -307,6 +307,30 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
307307
}
308308
}
309309

310+
unsigned int BuildCoresSetting::parse(const std::string & str) const
311+
{
312+
if (auto n = string2Int<decltype(value)>(str)) {
313+
if (*n == 0) {
314+
// Resolve 0 to getDefaultCores() at configuration time
315+
return settings.getDefaultCores();
316+
} else {
317+
return *n;
318+
}
319+
} else {
320+
throw UsageError("configuration setting '%s' should be an integer", name);
321+
}
322+
}
323+
324+
void BuildCoresSetting::assign(const unsigned int & v)
325+
{
326+
if (v == 0) {
327+
// Resolve 0 to getDefaultCores() at assignment time
328+
value = settings.getDefaultCores();
329+
} else {
330+
value = v;
331+
}
332+
}
333+
310334

311335
static void preloadNSS()
312336
{

src/libstore/include/nix/store/globals.hh

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,27 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
3333
unsigned int parse(const std::string & str) const override;
3434
};
3535

36+
struct BuildCoresSetting : public BaseSetting<unsigned int>
37+
{
38+
BuildCoresSetting(Config * options,
39+
unsigned int def,
40+
const std::string & name,
41+
const std::string & description,
42+
const StringSet & aliases = {},
43+
const bool documentDefault = true)
44+
: BaseSetting<unsigned int>(def, documentDefault, name, description, aliases)
45+
{
46+
options->addSetting(this);
47+
}
48+
49+
unsigned int parse(const std::string & str) const override;
50+
51+
void assign(const unsigned int & v) override;
52+
53+
// Inherit assignment operators from BaseSetting
54+
using BaseSetting<unsigned int>::operator=;
55+
};
56+
3657
const uint32_t maxIdsPerBuild =
3758
#ifdef __linux__
3859
1 << 16
@@ -43,8 +64,6 @@ const uint32_t maxIdsPerBuild =
4364

4465
class Settings : public Config {
4566

46-
unsigned int getDefaultCores();
47-
4867
StringSet getDefaultSystemFeatures();
4968

5069
StringSet getDefaultExtraPlatforms();
@@ -57,6 +76,8 @@ public:
5776

5877
Settings();
5978

79+
unsigned int getDefaultCores() const;
80+
6081
Path nixPrefix;
6182

6283
/**
@@ -151,9 +172,9 @@ public:
151172
)",
152173
{"substitution-max-jobs"}};
153174

154-
Setting<unsigned int> buildCores{
175+
BuildCoresSetting buildCores{
155176
this,
156-
getDefaultCores(),
177+
0,
157178
"cores",
158179
R"(
159180
Sets the value of the `NIX_BUILD_CORES` environment variable in the [invocation of the `builder` executable](@docroot@/language/derivations.md#builder-execution) of a derivation.
@@ -172,9 +193,7 @@ public:
172193
>
173194
> The number of parallel local Nix build jobs is independently controlled with the [`max-jobs`](#conf-max-jobs) setting.
174195
)",
175-
{"build-cores"},
176-
// Don't document the machine-specific default value
177-
false};
196+
{"build-cores"}};
178197

179198
/**
180199
* Read-only mode. Don't copy stuff to the store, don't change

tests/functional/build-cores.nix

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
with import ./config.nix;
2+
3+
{
4+
# Test derivation that checks the NIX_BUILD_CORES environment variable
5+
testCores = mkDerivation {
6+
name = "test-build-cores";
7+
buildCommand = ''
8+
echo "$NIX_BUILD_CORES" > $out
9+
'';
10+
};
11+
}

tests/functional/build-cores.sh

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/usr/bin/env bash
2+
3+
source common.sh
4+
5+
clearStoreIfPossible
6+
7+
echo "Testing build-cores configuration behavior..."
8+
9+
# Test 1: When build-cores is set to a non-zero value, NIX_BUILD_CORES should have that value
10+
echo "Testing build-cores=4..."
11+
rm -f "$TEST_ROOT"/build-cores-output
12+
nix-build --cores 4 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output
13+
result=$(cat $(readlink "$TEST_ROOT"/build-cores-output))
14+
if [[ "$result" != "4" ]]; then
15+
echo "FAIL: Expected NIX_BUILD_CORES=4, got $result"
16+
exit 1
17+
fi
18+
echo "PASS: build-cores=4 correctly sets NIX_BUILD_CORES=4"
19+
rm -f "$TEST_ROOT"/build-cores-output
20+
21+
# Test 2: When build-cores is set to 0, NIX_BUILD_CORES should be resolved to getDefaultCores()
22+
echo "Testing build-cores=0..."
23+
nix-build --cores 0 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output
24+
result=$(cat $(readlink "$TEST_ROOT"/build-cores-output))
25+
if [[ "$result" == "0" ]]; then
26+
echo "FAIL: NIX_BUILD_CORES should not be 0 when build-cores=0"
27+
exit 1
28+
fi
29+
echo "PASS: build-cores=0 resolves to NIX_BUILD_CORES=$result (should be > 0)"
30+
rm -f "$TEST_ROOT"/build-cores-output
31+
32+
echo "All build-cores tests passed!"

tests/functional/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ suites = [
145145
'placeholders.sh',
146146
'ssh-relay.sh',
147147
'build.sh',
148+
'build-cores.sh',
148149
'build-delete.sh',
149150
'output-normalization.sh',
150151
'selfref-gc.sh',

0 commit comments

Comments
 (0)