| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
A significant fraction of all created attribute sets are empty; hence
this is an easy optimisation to make.
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: I0884194d04c1ee95b2b239a253515f2152bc0856
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1179
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
|
|
This reverts parts of the CLs splitting the backing implementation for
Bindings and moves back to only the BTreeMap-backed implementation.
Our evaluation has indicated that the Vector-backed implementation
does not match the performance of the plain array used upstream, and
in my view the complexity introduced by it is not worth the relatively
small (single-digit percentage) performance increase with a
pivot-point close to the number of attributes yielded by
stdenv.mkDerivation.
Going forward we will trial implementations of attribute sets backed
by HAMTs, and investigate other mechanisms of speeding up the language.
Some changes from the previous CLs are retained, for example the
removal of insert_or_assign and the passing of capacity.
Change-Id: I6eb4b075b453949583360755055c21a29d7ff642
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1172
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
Since one of the two implementations essentially uses the same shape
as the upstream Bindings, we backport their merge sort implementation
to ensure that we're doing the same thing semantically.
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: I0d865897991eec0c4dd84d9bd0415cd1ca437792
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1162
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
|
|
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: Ief2e59d461452ce599abc63f6ebcfa07a7062491
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1161
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
|
|
We accidentally returned the incremented iterator in the
post-increment, this fixes it.
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: I36c79eb56359bb12a78ad3489e7d7d2eb2053510
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1140
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
|
|
This function in never called, so let's just remove it
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Change-Id: I79125866254d90dd0842bc86830d2103ac313cb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1125
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: isomer <isomer@tvl.fyi>
|
|
To aid in making the decision of where to (currently just statically)
use a vector or btree as the backing implementation, add an extra
constructor argument to Bindings::NewGC for a capacity, and use
a (currently hardcoded at 32, for no good reason other than it felt like
a reasonable number) pivot to switch between our possible backing
implementations. Then, update all the call sites where it feels
reasonable that we know the capacity statically to *pass* that capacity
to the constructor.
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: I1858c161301a1cd0e83aeeb9a58839378869e71d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1124
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: isomer <isomer@tvl.fyi>
|
|
Add an alternative impl of the now-abstract Bindings base class that is
backed by a std::vector, somewhat similar but stylistically a little
superior to the array-backed implementation in upstream nix. The
underlying iterator type in BindingsIterator is now backed by a
std::variant that we std::visit an overload over in order to implement
the various bits of the iterator interface.
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: I7fbd1f4d5c449e2f9b82102a701b0bacd5e80672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1123
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
|
|
To pave the way for the thing we want to do eventually which is use a
linear-time array for bindings (aka attribute sets) that are statically
known to be small enough to get a performance benefit from doing so,
make the Bindings class abstract, and define a BTreeBindings class that
inherits from it and is (currently always) returned from the static
initializer. The idea is that we'll have an ArrayBindings class as well
later that we can dispatch to conditionally based on an optional
"capacity" parameter or something like that.
There was some difficulty here in getting the iterator to work - the
approach we settled on ended up making a concrete BindingsIterator class
which will wrap a std::variant of either a btree iterator or something
else later, but right now just wraps a btree iterator.
Paired-With: Luke Granger-Brown <git@lukegb.com>
Paired-With: Vincent Ambo <mail@tazj.in>
Paired-With: Perry Lorier <isomer@tvl.fyi>
Change-Id: Ie02ca5a1c55e8ebf99ab1e957110bd9284278907
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1121
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
|
|
Change-Id: I1cfbb7e933da54198115b28ac509b0d04870fd8f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1127
Tested-by: BuildkiteCI
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
|
|
Having a default constructor for this causes a variety of annoying
situations across the codebase in which this is initialised to an
unexpected value, leading to constant guarding against those
conditions.
It turns out there's actually no intrinsic reason that this default
constructor needs to exist. The biggest one was addressed in CL/1138
and this commit cleans up the remaining bits.
Change-Id: I4a847f50bc90e72f028598196592a7d8730a4e01
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1139
Reviewed-by: isomer <isomer@tvl.fyi>
Tested-by: BuildkiteCI
|
|
nix:AttrName was one of the few classes that relied on the default
constructor of nix::Symbol (which I am trying to remove in a separate
change).
The class essentially represents the name of an attribute in a set,
which is either just a string expression or a dynamically evaluated
expression (e.g. string interpolation).
Previously it would be constructed by only setting one of the fields
and defaulting the other, now it is an explicit std::variant.
Note that there are several code paths where not all eventualities are
handled and this code is bug-for-bug compatible with those, except
that unknown conditions (which should never work) are now throwing
instead of silently doing ... something.
The language tests pass with this change, and the depot derivations
that I tested with evaluated successfully.
Change-Id: Icf1ee60a5f8308f4ab18a82749e00cf37a938a8f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1138
Reviewed-by: edef <edef@edef.eu>
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
|
|
These bits are no longer required with the hashmap-backed
implementation of attribute sets.
Change-Id: I8b936d8d438a00bad4ccf8e0b4dd719c559ce8c2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/912
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
|
|
We don't want traces compiled out since they're an actual language
feature that're used in userspace - also their absence is breaking the
tests
Change-Id: Icaefca8f52e94001785f724fdc0c10a7586b24e7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/562
Reviewed-by: lukegb <lukegb@tvl.fyi>
Reviewed-by: Kane York <rikingcoding@gmail.com>
Reviewed-by: lukegbot <bot@lukegb.com>
|
|
Also fixes the pkgconfig files to use the corresponding CMake variables.
Change-Id: I8095b8aff39ad91e592f3edc95555c9f1f1f153d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/545
Reviewed-by: tazjin <mail@tazj.in>
|
|
This ensures that we install both glog's .a and all the .so files we
generate into a single consistent output lib path (which is, err,
lib64, but whatever).
Change-Id: Ib6ac6eacf5f56e4b719cfb586db731efc122c31b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/544
Reviewed-by: tazjin <mail@tazj.in>
|
|
Change-Id: I87eb6e59782d720015d351d8829dc7b8688e01f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/543
Reviewed-by: tazjin <mail@tazj.in>
|
|
Completes the switch from Meson to CMake for the core build system in
Nix.
Meson was added originally because someone else had already done the
work for integrating it in Nix and it was an upgrade from the previous
setup.
However over time it became clear that Meson is not quite mature
enough for projects like Nix that have occasionally peculiar
configuration constraints.
Some issues encountered with Meson (some of these are due to the Meson
setup in Nix):
* Difficulty with generating correct compile_commands.json for
external tools like clangd
* Difficulty linking to libc++ when using clang
* Ugly shell invocations for certain parts of the build system (I want
these to be gone!!!)
This CMake setup mimics the Meson configuration, but there are some
differences (some temporary):
* headers are now included separately for each library (see a previous
commit that changes includes appropriately)
* autoheaders-style configuration is currently hardcoded. Before
blindly copying this I want to evaluate how much of it actually exists
for portability concerns that I don't have (such as support for OS
X).
* Nix is built with libc++ by default.
* [libstore] SQL schema is now inlined via a generated header, not an
included string literal
Abseil is still built as part of this build, rather than an external
dependency, because it chokes on differently configured compiler
invocations.
Note that because of the move to libc++ an unwanted behaviour is
introduced: glog log messages no longer have a body. I have yet to
debug what is going on there.
|
|
|
|
Previously all includes were anchored in one global mess of header
files. This moves the includes into filesystem "namespaces" (if you
will) for each sub-package of Nix.
Note: This commit does not introduce the relevant build system changes.
|
|
This function was a custom (and inefficient in the case of
single-character delimiters) string splitter which was used all over
the codebase. Abseil provides an appropriate replacement function.
|
|
Uses the equivalent absl::StartsWith and absl::EndsWith functions
instead.
|
|
Replaces these functions with corresponding functions from Abseil,
namely absl::StripAsciiWhitespace and absl::SimpleAtoi.
In the course of doing this some minor things I encountered along the
way were also refactored.
This also changes the signatures of the various custom readFile
functions to use absl::string_view types.
|
|
It is considered bad form to use things from includes in headers, as
these directives propagate to everywhere else and can make it
confusing.
types.hh (which is includes almost literally everywhere) had some of
these directives, which this commit removes.
|
|
|
|
... this fixes nixpkgs eval!
|
|
|
|
Without this alias, the garbage-collecting allocator won't be used and
allocated attribute set values won't be visible during GC.
|
|
|
|
These were things that took me a moment to realise.
|
|
Replaces the previous implementations which performed sorting with one
that instead walks through the map (which is already sorted) and
yields values from it.
This fixes a handful of language tests because the previous
implementation did not actually yield useful values on the new implementation.
|
|
|
|
In the change to the backing structure of attribute sets, the
requirement to manually balance the capacity of the structure went
away.
This is a) because Abseil's data structures manage this on their own,
and b) because the new Bindings class is allocated using `new (GC)`
rather than writing into a predefined memory area.
As part of this change functions related to the capacity were
deprecated and set to 0 values, which in turn caused the creation of
new attribute sets to return the same (mutable!) default value in
various cases, leading to "side effects" that caused evaluation
failures.
FWIW, I'm not sure if this optimisation had noticeable performance
impact, but while untangling libexpr it definitely doesn't help trying
to follow what it's doing - so bye, bye!
|
|
|
|
This feature does not appear in nixpkgs, so I don't care about it. My
only goal is evaluating nixpkgs.
|
|
Reading more through the old code, it seems like the intention
/sometimes/ is to replace values.
|
|
This will make all Attr values visible to the GC.
|
|
This is closer to bug-for-bug compatibility with the previous version,
which would put new elements at the end of the array and (due to the
linear scan) return previous ones.
|
|
|
|
EvalState::allocBindings had little to do with Bindings, other than
returning them, and didn't belong in that class.
|
|
|
|
This function does nothing anymore since the attributes are always
in-order.
|
|
The new attribute set API uses the iterators of the btree_map
directly. This requires changes in various files because the internals
of libexpr are very entangled.
This code runs and compiles, but there is a bug causing empty
attribute sets to be assigned incorrectly.
|
|
Instead of using a custom Args* iterator, use the one belonging to the
map type directly.
|
|
Instead of doing some sort of inline merge-sort of the two attribute
sets, use the attribute sets merge function.
This commit alone does not build and is not supposed to.
|
|
This is the first step towards replacing the implementation of
attribute sets with an absl::btree_map.
Currently many access are done using array offsets and pointer
arithmetic, so this change is currently causing Nix to fail in various
ways.
|
|
Replaces most uses of `string` with `std::string`.
This came up because I removed the "types.hh" import from
"symbol-table.hh", which percolated through a bunch of files where
`string` was suddenly no longer defined ... *sigh*
|
|
|
|
The functions in SymbolTable have been renamed to match the Google
Style guide, and some debug-only functions have been removed.
|
|
This replaces the previous use of std::unordered_set with
absl::node_hash_set.
This type was chosen because the current implementation requires
pointer stability.
This does not yet touch the 'Attr' struct.
As a bonus, the implementation of the SymbolTable struct is now
consolidated into a single header/implementation file pair.
|
