As it turns out using `std::regex` is actually the bottleneck
for root discovery. Just substituting `std::` -> `boost::`
makes root discovery twice as fast (3x if counting only userspace time).
Some rather ad-hoc measurements to motivate the switch:
(On master)
```
nix build github:nixos/nix/1e822bd4149a8bce1da81ee2ad9404986b07914c#nix-cli --out-link result-1e822bd4149a8bce1da81ee2ad9404986b07914c
taskset -c 2,3 hyperfine "result-1e822bd4149a8bce1da81ee2ad9404986b07914c/bin/nix store gc --dry-run --max 0"
Benchmark 1: result-1e822bd4149a8bce1da81ee2ad9404986b07914c/bin/nix store gc --dry-run --max 0
Time (mean ± σ): 481.6 ms ± 3.9 ms [User: 336.2 ms, System: 142.0 ms]
Range (min … max): 474.6 ms … 487.7 ms 10 runs
```
(After this patch)
```
taskset -c 2,3 hyperfine "result/bin/nix store gc --dry-run --max 0"
Benchmark 1: result/bin/nix store gc --dry-run --max 0
Time (mean ± σ): 254.7 ms ± 9.7 ms [User: 111.1 ms, System: 141.3 ms]
Range (min … max): 246.5 ms … 281.3 ms 10 runs
```
`boost::regex` is a drop-in replacement for `std::regex`, but much faster.
Doing a simple before/after comparison doesn't surface any change in behavior:
```
result/bin/nix store gc --dry-run -vvvvv --max 0 |& grep "got additional" | wc -l
result-1e822bd4149a8bce1da81ee2ad9404986b07914c/bin/nix store gc --dry-run -vvvvv --max 0 |& grep "got additional" | wc -l
```
(cherry picked from commit 3a1301cd6d)
Leverage #10766 to show how we can now resolve a store configuration
without actually opening the store for that resolved configuration.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
The existing header is a bit too big. Now the following use-cases are
separated, and get their own headers:
- Using or implementing an arbitrary store: remaining `store-api.hh`
This is closer to just being about the `Store` (and `StoreConfig`)
classes, as one would expect.
- Opening a store from a textual description: `store-open.hh`
Opening an aribtrary store implementation like this requires some sort
of store registration mechanism to exists, but the caller doesn't need
to know how it works. This just exposes the functions which use such a
mechanism, without exposing the mechanism itself
- Registering a store implementation: `store-registration.hh`
This requires understanding how the mechanism actually works, and the
mechanism in question involves templated machinery in headers we
rather not expose to things that don't need it, as it would slow down
compilation for no reason.
This patch finally applies the transition to std::less<>,
which is a transparent comparator. There's no functional
change and string lookups in sets are now more efficient
and don't produce temporaries (e.g. set.find(std::string_view{"key"})).
Unfortunately Feature is just an alias to `std::string`
and not a new-type, so a ton of code relies on it being
exactly a `std::string`.
Using transparent comparators just for StringSet necessitates
using it here as well.
The intention is to switch to transparent comparators from N3657 for
ordered set containers for strings and using the alias consistently
would simplify things.
When a PUT is redirected, some of the data can be sent by curl before headers are read. This means the subsequent PUT operation needs to seek back to origin.
Since we dropped fs::symlink_exists, we no longer have a need for the fs
namespace. Having less abstractions makes it easier to lookup the
functions in reference documentations.
As it turns out the orignal implementation of symlink_exists cannot be
used in Nix because it did now std::filesystem::filesystem_error.
The new implementation fixes that but is now actually the same as
pathExists except for the path type.
This name is close to the Nixpkgs lib function `escapeShellArg`,
making it easier to find.
A friendlier function with the same behavior as lib could be added
later.
Fixes
../src/libstore/unix/build/derivation-builder.cc:1130:86: warning: the compiler can assume that the address of ‘nix::DerivationBuilderParams::drv’ will never be NULL [-Waddress]
1130 | if (useChroot && settings.preBuildHook != "" && dynamic_cast<const Derivation *>(&drv)) {
| ^~~~
Assuming this check was left over from the time `drv` could be a
`BasicDerivation`.
I split it out before to try to separate the building logic, but now we
have the much better `DerivationBuilder` abstraction for that. With that
change, I think `LocalDerivationGoal` has outlived its usefulness.
We just inline it back into `DerivationGoal`, and do so with minimal
`#ifdef` for Windows.
Note that the order of statements in `~DerivationGoal` is different than
it was after the `~LocalDerivationGoal` split, but it is *restored* to
the way it original was before --- evidently I did the split slightly
wrong, but nobody noticed, probably because the order doesn't actually
matter.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Now that `DerivationBuilder` is created after the underlying data has
already been initialized, we can just refer this data normally, with a
direct reference.
Only `parsedDrv` takes a (borrowing) pointer, because independent of
initialization the derivation may or may not have structured attrs.
