Commit graph

1180 commits

Author SHA1 Message Date
Vincent Ambo
abff828ccc refactor(tvix/eval): remove use of imbl::OrdMap
Removes imbl::OrdMap in favour of an Rc over the standard library's BTreeMap,
which allows us to drop the imbl dependency completely.

In my local tests this is actually slightly faster for `hello` and `firefox`.

Change-Id: Ic9597ead4e98bf9530f290c6a94a3c5c3efd0acc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12201
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-08-19 11:02:50 +00:00
Vincent Ambo
adf9b4c54a refactor(tvix/eval): remove use of imbl::Vector
This vector type has served us well for now, but it contains internal refcounts
which are incompatible with upcoming changes related to garbage collection.

The performance impact of this change within all benchmarks I ran was within the
margin of error:

[nix-shell:/tmp/perf]$ hyperfine "./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings"
Benchmark 1: ./u64 -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
  Time (mean ± σ):      7.528 s ±  0.272 s    [User: 6.578 s, System: 0.631 s]
  Range (min … max):    7.160 s …  8.012 s    10 runs

nix-shell:/tmp/perf]$ hyperfine "./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings"
Benchmark 1: ./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
  Time (mean ± σ):      7.515 s ±  0.178 s    [User: 6.508 s, System: 0.652 s]
  Range (min … max):    7.276 s …  7.861 s    10 runs

Change-Id: Ib95f871956e336a1e5771f6293583854b1efb276
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12197
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-08-19 11:02:50 +00:00
Vincent Ambo
d6c57eb957 refactor(tvix/eval): ensure VM operations fit in a single byte
This replaces the OpCode enum with a new Op enum which is guaranteed to fit in a
single byte. Instead of carrying enum variants with data, every variant that has
runtime data encodes it into the `Vec<u8>` that a `Chunk` now carries.

This has several advantages:

* Less stack space is required at runtime, and fewer allocations are required
  while compiling.
* The OpCode doesn't need to carry "weird" special-cased data variants anymore.
* It is faster (albeit, not by much). On my laptop, results consistently look
  approximately like this:

  Benchmark 1: ./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
  Time (mean ± σ):      8.224 s ±  0.272 s    [User: 7.149 s, System: 0.688 s]
  Range (min … max):    7.759 s …  8.583 s    10 runs

  Benchmark 2: ./after -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
  Time (mean ± σ):      8.000 s ±  0.198 s    [User: 7.036 s, System: 0.633 s]
  Range (min … max):    7.718 s …  8.334 s    10 runs

  See notes below for why the performance impact might be less than expected.
* It is faster while at the same time dropping some optimisations we previously
  performed.

This has several disadvantages:

* The code is closer to how one would write it in C or Go.
* Bit shifting!
* There is (for now) slightly more code than before.

On performance I have the following thoughts at the moment:

In order to prepare for adding GC, there's a couple of places in Tvix where I'd
like to fence off certain kinds of complexity (such as mutating bytecode, which,
for various reaons, also has to be part of data that is subject to GC). With
this change, we can drop optimisations like retroactively modifying existing
bytecode and *still* achieve better performance than before.

I believe that this is currently worth it to pave the way for changes that are
more significant for performance.

In general this also opens other avenues of optimisation: For example, we can
profile which argument sizes actually exist and remove the copy overhead of
varint decoding (which does show up in profiles) by using more adequately sized
types for, e.g., constant indices.

Known regressions:

* Op::Constant is no longer printing its values in disassembly (this can be
  fixed, I just didn't get around to it, will do separately).

Change-Id: Id9b3a4254623a45de03069dbdb70b8349e976743
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12191
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-08-19 11:02:50 +00:00
Aspen Smith
5382cbb93a refactor(tvix/eval): Pull context out into its own module
I'm gonna be doing some poking around in the internals of Context, so in
preparation this pulls it out into its own module.

Change-Id: I72ea7df80b5f36f838934ee07bdba66874c334c9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12189
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-08-18 19:49:14 +00:00
Brian Olsen
39ef171e00 chore(tvix): Retain original passthru in Rust builds
When using the runTests feature of crate2nix the derivation that runs the
tests is put into passthru.test but all default.nix files for Rust crates
in Tvix threw that away.

This commit retains passthru so that you can get access to the test
derivation.

Change-Id: I8b7b7db57a49069348f08c12c00a3b1a41a0c05b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12215
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-08-15 23:37:00 +00:00
Aspen Smith
1f5a85740e chore(tvix/eval): Update module comment for value::string
This sentence is a little stale; let's just link to NixString directly
for the authoritative source of truth.

Change-Id: I64e065c4148d29702b09820a0e7724a65fae7c67
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12181
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-08-11 16:41:34 +00:00
Aspen Smith
e176e1f11a feat(tvix/value): Print derivation values specially
Just like tvix-repl does (except we don't force values when printing
them, so... not entirely like tvix-repl does). But it's something.

Change-Id: I2e69b08d7d82b0b2d337f1d4c5d87ed28475fa84
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12180
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
2024-08-11 16:41:34 +00:00
Aspen Smith
7c2ac040db chore(tvix/eval): Drop obsolete todo
the answer is at https://cl.tvl.fyi/c/depot/+/10798

Change-Id: I5f0ed51a3954c7241ef15a8268e0e51695e994c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12175
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-08-10 19:18:40 +00:00
Aspen Smith
d378111d77 feat(tvix/eval): Store hash in key of interner
Rather than storing the leaked allocation for the string as the key in
the interner, store the hash (using NoHashHashBuilder). I thought this
would improve performance, but it doesn't:

hello outpath           time:   [736.85 ms 748.42 ms 760.42 ms]
                        change: [-2.0754% +0.4798% +2.7096%] (p = 0.72 > 0.05)
                        No change in performance detected.

but it at least doesn't *hurt* performance, and it *does* avoid an
`unsafe`, so it's probably net good.

Change-Id: Ie413955bdb6f04b1f468f511e5ebce56e329fa37
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12049
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
2024-08-10 15:28:30 +00:00
Ilan Joselevich
e505880fde feat(tvix/eval): Re-enable testing for eval-okay-readDir
When we moved to building tvix with crate2nix we had to disable the
readDir tests for the same reasons mentioned in
https://cl.tvl.fyi/c/depot/+/12131. But now with the 12131 CL, we can
re-enable the readDir tests.

In this change I re-enabled the readDir tests in nix_tests by moving
them out of notyetpassing and I also deleted them from tvix_tests
because we don't need them in both places.

Change-Id: I82ac39605299a8b22d80f8b51fc8ec2476d21dc9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12133
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
2024-08-09 14:56:57 +00:00
Ilan Joselevich
3511e328ec feat(tvix/eval): Implement builtins.readFileType
builtins.readFileType was added to Nix back in version 2.14.

The tests were also moved out of notyetpassing in addition to the
readDir fixtures they depend on.

I caught a bug where we previously used std::fs::metadata (via the
.metadata() method on File) which follows symlinks so it would always
return false for is_symlink(). Instead we now use
std::fs::symlink_metadata directly which does not follow symlinks, so
tests now pass. This wasn't an issue for builtins.readDir as it uses
walkdir and walkdir doesn't follow symlinks either.

Change-Id: I58eb97bdb5ec95df4f6882f495f8c572fe7c6793
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12130
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
Tested-by: BuildkiteCI
2024-08-09 14:35:57 +00:00
Ilan Joselevich
c554c1c1c0 feat(tvix/eval): Implement Display for io::FileType
In newer versions of Nix there's a builtins.readFileType builtin, we
should try to avoid duplicating the enum -> string conversion by
implementating Display before we implement builtins.readFileType.

Change-Id: I579e95949a76eb33d2e7bda0000ed84859df765e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12129
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
Tested-by: BuildkiteCI
2024-08-09 14:03:46 +00:00
Ilan Joselevich
f648f17ec3 feat(tvix): Jemalloc -> MiMalloc
Use the faster and newer MiMalloc memory allocator for all endpoints in
the workspace.

Change-Id: Ic60237284ed168e46ec6e8f28e2710bae4385c6f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12149
Tested-by: BuildkiteCI
Reviewed-by: aspen <root@gws.fyi>
2024-08-08 08:06:23 +00:00
Aspen Smith
6559ab4cf5 feat(tvix/eval): Put interner in a thread-local RefCell
Rather than making the interner be a global lazy_static mutex, put it in
a thread-local RefCell. This doesn't change anything in terms of
sharing (since we're currently actually just single threaded), but
avoids the overhead of a mutex, for a nice performance boost (compared
to the mutex version):

hello outpath           time:   [726.71 ms 729.79 ms 735.69 ms]
                        change: [-5.7277% -3.9733% -2.1144%] (p = 0.00 < 0.05)
                        Performance has improved.

Change-Id: I240b238dcbaf854ebafc3017b4425fb7d7b91b03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12048
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
2024-08-08 00:02:36 +00:00
Aspen Smith
6366cee717 feat(tvix/eval): Intern (and leak) small strings, behind a mutex
This is the most naive version of string interning possible - we store a
map from the string itself to the pointer behind a global mutex, and
memoize the allocation of all strings below a threshold length (16
bytes, for now) into that map. This requires leaking /all/ strings,
since it's not easy to know just from the pointer that a string has been
interned - so interning is disabled if string leaking is also disabled.

In the case where we're leaking strings (the default), even the naive
version of this gets us a pretty nice perfomance boost:

hello outpath           time:   [742.54 ms 745.89 ms 749.14 ms]
                        change: [-2.8722% -2.0135% -1.0654%] (p = 0.00 < 0.05)
                        Performance has improved.

However, in the case where we're not leaking strings, we have to keep
track of which strings have and haven't been interned, which makes this
a little worse:

hello outpath           time:   [779.30 ms 792.82 ms 808.74 ms]
                        change: [+2.5258% +4.0884% +5.8931%] (p = 0.00 < 0.05)
                        Performance has regressed.

Hopefully we can close the gap here a bit with some clever
tricks (coming next).

Change-Id: If08cb48ede703c7fe3bdd8d617443f8a561ad09b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12047
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
2024-08-08 00:02:36 +00:00
Aspen Smith
b8f92a6d53 feat(tvix/eval): Forbid Hash{Map,Set}, use Fx instead
Per https://nnethercote.github.io/perf-book/hashing.html, we have
basically no reason to use the default hasher over a faster,
non-DoS-resistant hasher. This gives a nice perf boost basically for
free:

hello outpath           time:   [704.76 ms 714.91 ms 725.63 ms]
                        change: [-7.2391% -6.1018% -4.9189%] (p = 0.00 < 0.05)
                        Performance has improved.

Change-Id: If5587f444ed3af69f8af4eead6af3ea303b4ae68
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12046
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Autosubmit: aspen <root@gws.fyi>
2024-08-07 12:38:40 +00:00
Yureka
1d7ba89c19 chore(tvix): change deprecated default_features to default-features
Change-Id: Ida31fe9a1205cdfc852d2ecbcbda32933b634562
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12143
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-08-07 11:36:04 +00:00
Aspen Smith
59056cf705 feat(tvix/eval): Leak strings (with flag to disable)
Default to always leaking strings, and copying strings by copying
pointers rather than cloning the underlying allocation. This (somewhat
bafflingly) doesn't seem to affect any benchmarks, but paves the way for
some tricks around string allocation that do.

Unfortunately, we can't do this (yet?) for contextful strings, for
reasons I don't currently understand but which I will address later,
when I address contextful strings more holistically.

I've left a flag in here to disable this, both to test the cloning logic
for strings for when/if we decide to bring this back, and to allow
people who care more about memory usage than perf to disable leaking.

Change-Id: Iec44bcbfe9b3d20389d2450b9a551792a79b9b26
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12045
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-08-05 10:49:27 +00:00
Aspen Smith
756539a596 chore(3p/sources): Bump channels & overlays (2024-07-28)
* Treewide: re-run depotfmt

* //third_party/nixpkgs:html5validator: build with Python 3.11,
  dependency openstackdocstheme doesn't support 3.12

* //users/sterni/machines/ingeborg: adapt to poorly handled fcgiwrap
  module API change: https://github.com/NixOS/nixpkgs/pull/318599

* //tvix/*-go: regenerate protobuf files

* //third_party/nixpkgs:treefmt: Remove patch for merged pull request

* //users/flokli/ipu6-softisp: rebase, drop upstreamed kernel patches

Change-Id: Ie4e0df007c287e8cd6207683a9a25838aa5bd39a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11971
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
2024-08-01 10:06:33 +00:00
Matthew Tromp
038d1dd551 feat(tvix/eval): ConstantIdx expansion for more ops
Previously, OpConstant would display some detail about its
ConstantIdx: whether it's a thunk or closure, and what its address
is. This has been expanded to also show when the ConstantIdx is a
blueprint, along with the blueprint's address, and to the other
opcodes that use a ConstantIdx.

Currently, it seems like blueprint addresses don't correspond to the
address of the thunk listed in the bytecode output, but it's still
useful to see that the constant being grabbed is a blueprint, and
maybe this pointer can be made to make more sense in the future.

Change-Id: Ia212b0d52b004c87051542c093274e7106ee08e4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12044
Tested-by: BuildkiteCI
Reviewed-by: aspen <root@gws.fyi>
Autosubmit: chickadee <matthewktromp@gmail.com>
2024-07-27 22:37:59 +00:00
Matthew Tromp
0b987a509f fix(tvix/eval): don't bubble up io errors from path_exists
path_exists was returning an error when certain common IO errors were
encountered. e.g. in the path "/dev/null/.", path_exists would throw
an error because the underlying call to Path::try_exists threw an
error because null isn't a directory. But if null isn't a directory,
then the path is invalid, so this should really be returning
false. That's what nix's behavior is and that's what makes sense.

The trait function isn't being changed because some other
implementers (e.g. tvix_store_io) have actual errors they can throw.

Fixes: b/411
Change-Id: I9e810e7a198bffe61365697c6d3d7e71f264c20b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12042
Tested-by: BuildkiteCI
Autosubmit: chickadee <matthewktromp@gmail.com>
Reviewed-by: aspen <root@gws.fyi>
2024-07-27 21:29:46 +00:00
Aspen Smith
89d204d295 feat(tvix/eval): Bump imbl to 3.0.0
Change-Id: I88053fa2df35dc486c51d015f4b2156541c07af3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11962
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-07-07 15:05:27 +00:00
Aspen Smith
8821746d6c fix(tvix/repl): Share globals and sourcemap across evaluations
Now that we can bind (potentially lazy, potentially lambda-containing)
values in the REPL and then reference them in subsequent evaluations,
it's important that the values to which we construct shared references
are shared across those subsequent evaluations - otherwise, we get
panics due to unknown source map locations, or dropped weak references
to globals.

This change assigns both the globals and the source map as fields on the
Repl after the first evaluation, and then passes those in (to the
EvaluationBuilder) on subsequent evaluations.

On the EvaluationBuilder side, there's some panicking introduced - this
is intentional, as my intent is for the builder to be configured
statically enough that panicking is the best way to report errors
here (it's always a bug to misconfigure an Evaluation, and we'd never
want to handle it dynamically).

Change-Id: I37225697235c22b683ca48a17d30fa8fedd12d1b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11960
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-07-07 15:04:26 +00:00
Aspen Smith
3a79f93795 refactor(tvix/eval): Construct globals in EvaluationBuilder::build
Construct the Rc<GlobalsMap> for the evaluation as part of
EvaluiationBuilder::build, rather than deferring it until we actually
compile. This changes nothing functionally, but gets us one step closer
to sharing this globals map across evaluations.

Change-Id: Id92e9fb88d974d763056d4f15ce61962ab776e84
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11957
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-07-06 15:24:50 +00:00
Aspen Smith
dfe137786c refactor(tvix/eval): Builderize Evaluation
Make constructing of a new Evaluation use the builder pattern rather
than setting public mutable fields. This is currently a pure
refactor (no functionality has changed) but has a few advantages:

- We've encapsulated the internals of the fields in Evaluation, meaning
  we can change them without too much breakage of clients
- We have type safety that prevents us from ever changing the fields of
  an Evaluation after it's built (which matters more in a world where we
  reuse Evaluations).

More importantly, this paves the road for doing different things with
the construction of an Evaluation - notably, sharing certain things like
the GlobalsMap across subsequent evaluations in eg the REPL.

Fixes: b/262
Change-Id: I4a27116faac14cdd144fc7c992d14ae095a1aca4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11956
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-07-06 15:03:46 +00:00
Aspen Smith
ac3d717944 feat(tvix/eval): Allow passing in an env to evaluation
Allow passing in a top-level env, a map from name to value, to
evaluation. The intent is to support bound identifiers in the REPL just
like upstream nix does.

Getting this working involves mucking around a bit with internals - most
notably, locals now only optionally have a Span (since locals don't have
an easy span we can use) - and getting that working requires propagating
some minor hacks to places where we currently *need* a span (and which
would require too much changing now to make spans optional; my guess is
that that would essentially end up making spans optional throughout the
codebase).

Also, some extra care has to be taken to close out the scope in the case
that we do pass in an env, to avoid breaking our assumptions about the
size of the stack when we return from the toplevel

Change-Id: Ie475b2d3dfc72ccbf298d2a3ea28c63ac877d653
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11953
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-07-05 16:39:34 +00:00
Aspen Smith
6037888e18 refactor(tvix/eval): Drop LightSpan entirely
This was made unnecessary in c92d06271 (feat(tvix/eval): drop
LightSpan::Delayed, 2023-12-08) because it didn't improve benchmarks as
much as expected and has been vestigial since; this continues the
cleanup by just removing it altogether

Change-Id: I21ec7ae9b52a5cccd2092696a5a87f658194d672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11949
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-07-05 03:53:44 +00:00
Florian Klink
afbe995aa9 chore(tvix): bump to data-encoding 2.6.0
Change-Id: I26af403bfa99e5d1cff24641a3dba908e1d06686
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11899
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
2024-07-01 06:57:01 +00:00
Florian Klink
aa7d125c12 refactor(tvix/eval): prefix ctx iterators in NixString
NixString::iter_plain() didn't make much sense, especially without a docstring.

Rename it to iter_ctx_plain(), and copy the docstring from NixContext.

Do the same for the two other context element types too.

Change-Id: I1bbfcb967d8d9b14487d069bfe3a1f762253ef4d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11882
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Reviewed-by: flokli <flokli@flokli.de>
2024-06-26 14:25:20 +00:00
Florian Klink
a0993e7304 feat(tvix/eval): add ErrorKind::UnexpectedArgumentBuiltin
Change-Id: Ieb091b32aad566719fbe8604c4a589f5ccaaf6b3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11877
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-06-26 04:51:31 +00:00
Florian Klink
970e2a045b refactor(tvix/eval): rename UnexpectedArgument{,Formals}
There's other places where unexpected arguments can be provided, like
in builtins.

Make space for that error type.

Change-Id: Ic831497a3a1dd36a3a184bedadcf1374bf0ae6db
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11876
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
2024-06-26 04:51:31 +00:00
Florian Klink
080654aaf9 feat(tvix/eval): add file_type method
This allows peeking of the type at a given path.

It's necessary, as an open() might not fail until you try to read()
from it, and generally, stat'ing can be faster in some cases.

Change-Id: Ib002da3194a3546ca286de49aac8d1022ec5560f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11871
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-06-26 04:51:31 +00:00
Ilan Joselevich
04f04cfc27 refactor(tvix/eval): drop placeholder impls of builtins in glue
The builtins `placeholder` and `filterSource` are now implemented in
tvix-glue and so we no longer need their placeholder "implementations" in
tvix-eval.

Change-Id: Ibf161ccf1ad40103e9fbb688e9f6be58e7772af1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11867
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
2024-06-22 21:50:30 +00:00
Florian Klink
5077ca70de chore(tvix/eval): move eval docs to tvix/docs
Change-Id: I75b33c43456389de6e521b4f0ad46d68bc9e98f6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11809
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-06-14 08:00:34 +00:00
binarycat
beb7f57c73 fix(tvix/eval): handle builtins.split matching the empty string
This prevents the following statements from looping endlessly:

```
builtins.split "(.*)" ""
builtins.split "([abc]*)" "abc"
builtins.split "(.*)" "abc"
builtins.split ".*" ""
```

Cover these (and some more examples) in the test suite.

Co-Authored-By: Florian Klink <flokli@flokli.de>
Change-Id: Ibd339f971e0f4e3e5c229816e2be5a8e3836fec9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11743
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-06-11 14:59:02 +00:00
Ben Webb
23e0973cdf docs(tvix): fix some typos across various documents
Fix some typos found while reading various documents, mostly those
relating to the castore.

Here is a summary of the edits.

- fix broken link between documents in the store and castore directories
- clarify expression in castore's data model document that indicates
  that the *name* of each child node of a directory must be unique
  across all three lists of children
- add missing closing parenthesis in castore's data model document
- replace "how" with "what" in the phrase "unclear how a ... would even
  look like" in castore's why-not-git-trees document
- remove unnecessary articles in castore's blobstore chunking document
- add missing "y" to "optionall" in eval's compilation of bindings
  document

Change-Id: I1997ea91bb4e9c40abcd81e0cde9405968580ba6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11763
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-06-08 21:17:56 +00:00
Aspen Smith
d0ab3c8d15 feat(tvix): Switch to jemalloc
Switch tvix to using jemalloc as the default global allocator on
supported (eg, non-msvc) platforms. This gives a pretty reasonable
performance boost basically for free:

int                     time:   [24.361 µs 24.386 µs 24.418 µs]
                        change: [-19.355% -18.859% -18.527%] (p = 0.00 < 0.05)
                        Performance has improved.

merge small attrs       time:   [37.201 µs 37.328 µs 37.442 µs]
                        change: [-24.609% -24.266% -23.982%] (p = 0.00 < 0.05)
                        Performance has improved.

merge large attrs with small attrs
                        time:   [20.030 ms 20.135 ms 20.251 ms]
                        change: [-4.2811% -3.2549% -2.3807%] (p = 0.00 < 0.05)
                        Performance has improved.

hello outpath           time:   [967.91 ms 974.07 ms 983.33 ms]
                        change: [-5.5008% -4.4552% -3.4099%] (p = 0.00 < 0.05)
                        Performance has improved.

Change-Id: I6c6e6e3295ec2fca01ea28dc37bcb201cd811767
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10851
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-06-06 10:29:33 +00:00
Ilan Joselevich
34d93f1d96 fix(tvix): make rstest pick up new test case files
Context: https://github.com/la10736/rstest/issues/256

Cargo will now cause a rebuild whenever a new test case file is added.
Previsouly running `cargo test` after adding a new test case resulted in
that case not being picked up and ignored.

Change-Id: Ibfc420b5bfe3f3ee41d3ebd3fb9d248819fa6ed9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11751
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
2024-06-05 17:52:17 +00:00
Aspen Smith
72b9a126b8 feat(tvix/glue): Implement builtins.storePath
This one's relatively simple - we just check if the store path exists,
and if it does we make a new contextful string containing the store path
as its only context element.

Automatic testing seems tricky for this (I think?) so I tested it
manually:

tvix-repl> builtins.storePath /nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1
=> "/nix/store/yn46i4xx5alh7gs6fpkxk430i34rp2q9-hello-2.12.1" :: string

Change-Id: I8a0d9726e4102ab872c53c2419679c2c855a5a18
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11696
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
2024-06-05 17:50:15 +00:00
Ilan Joselevich
1b39d5868a feat(tvix): add CI targets for checking crate features powerset
Closes: https://b.tvl.fyi/issues/401

With this change all crate features (and their combinations) will be built and
tested in CI.

From now on, when adding/removing a Cargo feature for a crate,
you will want to add it to the features power set that gets tested in CI.
For each crate there's a default.nix with a `mkFeaturePowerset` invocation,
modify the list to include/remove the feature.
Note that you don't want to add "collection" features,
such as `fs` for tvix-[ca]store or `default`.

Change-Id: I966dde1413d057770787da3296cce9c1924570e0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11717
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-06-03 16:35:51 +00:00
Ilan Joselevich
be624daf13 fix(tvix/eval): nix_tests.rs's eval_test requires impure flag
This fixes the following command:
'cargo test --no-default-features --features nix_tests'

Change-Id: I9883c39e1e428c72a0e7e0b75a73c8ed734abd3b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11740
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-05-30 21:52:16 +00:00
Florian Klink
5405ed9dc6 fix(tvix/eval): proptests require arbitrary feature
`cargo test --no-default-features` fails, if we don't conditionalize
this on the `arbitrary` feature too.

Change-Id: I81a277810119fed0cfc37c942c422f731aa14b2e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11726
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 16:18:30 +00:00
Florian Klink
baa39d9d09 fix(tvix/eval/tests/one_offs): test_source_builtin can be pure
`Evaluation::new_impure()` would require the test to be impure, but
there's nothing in this test specifically requiring us to make use of
impure features.

Change-Id: Idb24981195d1a94f51053ae04403eb5f0e27f3d9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11725
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 16:18:30 +00:00
Florian Klink
c83f2e765c fix(tvix/eval/tests/nix_oracle): tests are impure-only
This uses StdIO, which is only available when the `impure` feature is
enabled.

Change-Id: I039b1f45f6619dd099fa943e58322ff521482dfa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11724
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 16:18:30 +00:00
Florian Klink
2c628f6e1c refactor(tvix/eval): move nix_tests to separate module
Gate this behind the nix_tests feature flag (which wasn't applied
consistently).

Also, at least right now all of these use StdIO internally, either by
creating it on their own, or through the `eval_test` helper.

Once we have some proper classification system for tests we can probably
split this up further, but for now only run them if the impure feature
is enabled.

Change-Id: I3236cf09b3391585df99073367c4e4832c5e7898
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11723
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
2024-05-30 16:18:30 +00:00
Florian Klink
543c103903 fix(tvix/eval/vm/generators): allow unused rq_{path_exists,read_dir}
These VM requests are only emitted by code gated behind the impure feature.

To prevent warning from popping up when building without default
features, allow these to be unused if `impure` is not enabled.

Change-Id: Id871a5215e9a0f09aa78edecdd111369ee7ffe34
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11722
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
2024-05-30 16:18:30 +00:00
Florian Klink
6fb542aae6 fix(tvix/eval/nix_search_path): gate tests on impure feature
These use StdIO, which is only available if the impure feature is
enabled.

Change-Id: I18d8e191a7eba6ba5bd59f43631973eaa796c7bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11721
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
2024-05-26 19:53:51 +00:00
Florian Klink
db68c104af fix(tvix/eval/io): OsStringExt and std::fs::File import are impure-only
These are only needed when building with the impure feature enabled.

This removes some warnings when building with --no-default-features.

Change-Id: I3139d9133d4846aeb1b1b5f3830c0d078d047292
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11720
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2024-05-26 19:52:49 +00:00
edef
edd93b1962 fix(tvix/eval): drop superfluous string context check
cl/11712 simultaneously introduced this check and made it unnecessary,
since NixString::context should never return `Some` for empty contexts
now.

Change-Id: I41a655ff33910e8326cbb7d7526eb91bd19e9585
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11713
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2024-05-25 11:33:22 +00:00
edef
49750fa1e7 fix(tvix/eval): disallow empty but allocated string contexts
Both `Some(NixContext::new())` and `None` represent empty contexts,
but the former trips up `NixString::has_context`, and seems likely
to trip up other things.

We could hide the difference in the accessors, but we don't really
*want* the distinction to exist, since heap-allocating a null value
is pretty much always a mistake.

Change-Id: Ie84d26fb0d4b59e68354891ba13bde3bae40ab6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11712
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-05-25 08:42:32 +00:00