These counters are extremely expensive in a multi-threaded
program. For instance, disabling them speeds up evaluation of the
NixOS/nix/2.21.2 from 32.6s to 17.8s.
This has multiple dangling pointer issues that lead to segfaults in e.g.:
nix eval --expr '(builtins.getFlake "github:nixos/nixpkgs/25.05")' --impure
This reverts commit ad175727e4, reversing
changes made to d314750174.
A follow-up optimization will make it impossible to make a find function
that returns an iterator in an efficient manner. All consumer code can
easily use the `get` variant.
When doing multithreaded evaluation, we want to ensure that any Nix
file is parsed and evaluated only once. The easiest way to do this is
to rely on thunks, since those ensure locking in the multithreaded
evaluator. `fileEvalCache` is now a mapping from `SourcePath` to a
`Value *`. The value is initially a thunk (pointing to a
`ExprParseFile` helper object) that can be forced to parse and
evaluate the file. So a subsequent thread requesting the same file
will see a thunk that is possibly locked and wait for it.
The parser cache is gone since it's no longer needed. However, there
is a new `importResolutionCache` that maps `SourcePath`s to
`SourcePath`s (e.g. `/foo` to `/foo/default.nix`). Previously we put
multiple entries in `fileEvalCache`, which was ugly and could result
in work duplication.
These constant Values have no business being in the EvalState in the
first place. The ultimate goal is to get rid of the ugly `getBuiltins`
and its relience (in `createBaseEnv`) on these global constants is getting in the way.
Same idea as in f017f9ddd3.
Co-authored-by: eldritch horrors <pennae@lix.systems>
This object is always constant and will never get modified.
Having it as a global (constant) static is much easier and
unclutters the EvalState.
Same idea as in f017f9ddd3.
Co-authored-by: eldritch horrors <pennae@lix.systems>
Since the only construction and push_back() calls
to Bindings happen through the `BindingsBuilder` [1] we don't
need to keep `capacity` around on the heap anymore. This saves 8 bytes
(because of the member alignment padding)
per one Bindings allocation. This isn't that much, but it does
save significant memory.
This also shows that the Bindings don't necessarily have to
be mutable, which opens up opportunities for doing small bindings
optimization and storing a 1-element Bindings directly in Value.
For the following scenario:
nix-env --query --available --out-path --file ../nixpkgs --eval-system x86_64-linux
(nixpkgs revision: ddcddd7b09a417ca9a88899f4bd43a8edb72308d)
This patch results in reduction of `sets.bytes` 13115104016 -> 12653087640,
which amounts to 462 MB less bytes allocated for Bindings.
[1]: Not actually, `getBuiltins` does mutate bindings, but this is pretty
inconsequential and doesn't lead to problems.
In b70d22b `mkStringNoCopy()` was renamed to
`mkString()`, but this is a bit risky since in code like
vStringRegular.mkString("regular");
we want to be sure that the right overload is picked. (This is
especially problematic since the overload that takes an
`std::string_view` *does* allocate.) So let's be explicit.
(Rebased from https://github.com/NixOS/nix/pull/11551)
The motivation for this change is two-fold:
1. Commonly used Symbol values can be referred to
quite often and they can be assigned at compile-time
rather than runtime.
2. This also unclutters EvalState constructor, which was
getting very long and unreadable.
Spiritually similar to https://gerrit.lix.systems/c/lix/+/2218,
though that patch doesn't allocate the Symbol at compile time.
Co-authored-by: eldritch horrors <pennae@lix.systems>
Update src/libutil/windows/current-process.cc
Prefer `nullptr` over `NULL`
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/unix/current-process.cc
Prefer C++ type casts
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/windows/current-process.cc
Prefer C++ type casts
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/unix/current-process.cc
Don't allocate exception
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
This is needed to rearrange include order, but I also think it is a good
thing anyways, as we seek to reduce the use of global settings variables
over time.
* It is tough to contribute to a project that doesn't use a formatter,
* It is extra hard to contribute to a project which has configured the formatter, but ignores it for some files
* Code formatting makes it harder to hide obscure / weird bugs by accident or on purpose,
Let's rip the bandaid off?
Note that PRs currently in flight should be able to be merged relatively easily by applying `clang-format` to their tip prior to merge.
c39cc00404 has added assertions for
all Value accesses and the following case has started failing with
an `unreachable`:
(/tmp/fun.nix):
```nix
{a}: a
```
```
$ nix eval --impure --expr 'import /tmp/fun.nix {a="a";b="b";}'
```
This would crash:
```
terminating due to unexpected unrecoverable internal error: Unexpected condition in getStorage at ../include/nix/expr/value.hh:844
```
This is not a regression, but rather surfaces an existing problem, which previously
was left undiagnosed. In the case of an import `fun` is the `import` primOp, so that read is invalid
and previously this resulted in an access into an inactive union member, which is UB.
The correct thing to use is `vCur`. Identical problem also affected the case of a missing argument.
Add previously failing test cases to the functional/lang test suite.
Fixes#13448.
This factors out most of the value representation into a mixin class.
`finishValue` is now gone for good and replaced with a simple template
function `setStorage` which derives the type information/disriminator from
the type of the argument. Likewise, reading of the value goes through function
template `getStorage`.
An empty type `Null` is introduced to make the bijection InternalType <-> C++ type
complete.
`getPrimOp` function was basically identical to existing
`Value::primOpAppPrimOp` modulo some trivial differences.
Makes sense to reuse existing code for that.
