Commit graph

10 commits

Author SHA1 Message Date
Ryan Lahfa
54ad5a19e7 feat(tvix/cli): test firefox (drv|out)Path
With context strings, we are now able to reproduce up to firefox.

Until a more problematic example can be found, we should at least
lean on this particular example.

Change-Id: Ibeaf799d26e1f160b6c93f8ccd978702a2a7e3e1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10460
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2024-01-03 23:48:03 +00:00
Adam Joseph
29878259b3 feat(tvix/cli): add benchmark for bf286a54bc
cl/7558 used this expression as a benchmark to justify the introduction
of LightSpan::Delayed:

  builtins.length (builtins.attrNames (import ${pkgs.path} {}))

Let's add it as a benchmark case so it can be referenced easily.

Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}}
Change-Id: Idb6c69ddd284605dd3b5fd9ac5c79a69b9a470b7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10253
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-12 14:28:53 +00:00
Adam Joseph
0c15a09b15 feat(tvix/cli): add macrobenchmark
This commit adds a simple MVP benchmark, built on our nix
infrastructure instead of cargo.  It simply runs `tvix-eval` inside
of GNU time, and prints the three essential statistics in a short
JSON blob.

You can run the benchmark with a simple `nix run`, like:

  nix run -f . tvix.cli.benchmark-hello
  nix run -f . tvix.cli.benchmark-firefox
  nix run -f . tvix.cli.benchmark-cross-firefox

Currently these blobs are stored only in the CI logs, which I'm sure
get garbage-collected at some point.  We should be putting them in
the git trailers, but that can wait for a future CL.

I tried using `cargo bench` for this but found it incredibly
frustrating.  Maybe I'm doing it wrong.  It seems to be designed for
microbenchmarks only, and very hard to control.  It kept building
all sorts of unnecessary stuff (like the tests), and unlike
crate2nix it was doing all the builds on only a single machine
instead of using more than one machine.  Worse, for that single
machine it kept picking my laptop instead of my fast servers!  It
seems excessively cargo-flavored for such a straightforward task.

Benchmark: {"hello.outPath":{"kbytes":"244736","system":"0.36","user":"2.76"}}
Benchmark: {"firefox.outPath":{"kbytes":"1506736","system":"2.38","user":"32.01"}}
Benchmark: {"pkgsCross.aarch64-multiplatform.firefox.outPath":{"kbytes":"11334548","system":"10.70","user":"107.07"}}

Change-Id: I85bc046ec551360284d7ecfc81a03914f0085909
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10216
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-12-12 11:56:58 +00:00
sterni
4516cd09c5 fix(tvix/eval): only finalise formal arguments if defaulting
When dealing with a formal argument in a function argument pattern that
has a default expression, there are two different things that can happen
at runtime: Either we select its value from the passed attribute
successfully or we need to use the default expression. Both of these may
be thunks and both of these may need finalisers. However, in the former
case this is taken care of elsewhere, the value will always be finalised
already if necessary. In the latter case we may need to finalise the
thunk resulting from the default expression. However, the thunk
corresponding to the expression may never end up in the local's stack
slot. Since finalisation goes by stack slot (and not constants), we need
to prevent a case where we don't fall back to the default expression,
but finalise anyways.

Previously, we worked around this by making `OpFinalise` ignore
non-thunks. Since finalisation of already evaluated thunks still
crashed, the faulty compilation of function pattern arguments could
still cause a crash.

As a new approach, we reinstate the old behavior of `OpFinalise` to
crash whenever encountering something that is either not a thunk or
doesn't need finalisation. This can also help catching (similar)
miscompilations in the future. To then prevent the crash, we need to
track whether we have fallen back or not at runtime. This is done using
an additional phantom on the stack that holds a new `FinaliseRequest`
value. When it comes to finalisation we check this value and
conditionally execute `OpFinalise` based on its value.

Resolves b/261 and b/265 (partially).

Change-Id: Ic04fb80ec671a2ba11fa645090769c335fb7f58b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8705
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
2023-06-20 10:07:44 +00:00
sterni
385c797884 fix(tvix/eval): thunk unary operator applications
Unary operator applications are thunked which can easily be observed by

  nix-instantiate --eval -E '[ (!true) (-1) ]'

Unfortunately, there are few simple expressions where this makes a
difference in the end result. Thus it only cropped up when using nixpkgs
for cross compilation: Here we would compile the expression

  !(stdenv.cc.isGNU or false)

to assemble python3Minimal's passthru attribute set (at least this seems
to be the most likely explanation from the backtraces I've studied).
This means that an unthunked

    <stdenv.cc.isGNU or false>
    OpForce
    OpInvert

would be performed in order to assemble this attribute set, causing
stdenv.cc to be evaluated too early, causing an infinite recursion.

Resolves b/273.

It seems that having a test suite that doesn't use --strict and relies
on thunks rendered as <CODE> would be beneficial for catching such
issues. I've not been able to find a test case with --strict that
demonstrates the problem fixed in this CL.

Change-Id: I640a5827b963f5b9d0f86fa2142e75e3a6bbee78
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8654
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-29 12:44:08 +00:00
Florian Klink
4b00f1d7ac feat(tvix/cli): check nixpkgs eval for pkgs.hello.outPath
We already evaluate this the same as Nix does, so worth adding it into
CI.

Change-Id: I529faccac6e5e16e0bc985ab4c3e0cd07bb23293
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8624
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
2023-05-24 09:17:42 +00:00
Florian Klink
1fb5008237 feat(tvix/cli): introduce mkNixpkgsEvalCheck, add outPath check
This introduces a function that can be instantiated with an attribute
path to instantiate, as well as the expected path (normally interpolated
with the nix evaluator).

Check both pkgs.stdenv.drvPath and pkgs.stdenv.outPath to match.

Change-Id: Id667ed35fa159ff83fedb3017ef8d3271aa42695
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8606
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-05-24 09:17:41 +00:00
Florian Klink
8cfd303203 chore(tvix/cli): don't set COREPKGS anymore
Since cl/8213, tvix bundles corepkgs.

Change-Id: I17fa4452a6a1d554ae67b4aed9d9b1e77cb495f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8305
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2023-03-14 20:50:01 +00:00
Vincent Ambo
0dd07440b4 test(tvix/cli): eval nixpkgs.stdenv in CI and assert that it matches
Change-Id: If80194b5fdbf69512217bd4780e416e678045323
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8023
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
2023-02-03 10:47:18 +00:00
Vincent Ambo
d9e2bec953 refactor(tvix): split binary (REPL etc.) out from evaluator library
The tvix-eval project is independent from any *uses* of the evaluator,
such as the tvix-repl.

This functionality has been split out into a separate "tvix-cli"
crate. Note that this doesn't have to mean that this CLI crate is the
"final" CLI crate for tvix, the point of this is not "getting the CLI
structure right" but rather "getting the evaluator structure right".

This reshuffling is part of restructuring the way that functionality
like store communication is injected into language evaluation.

Note that at this commit the new CLI crate is not at feature-parity.

Change-Id: Id0af03dc8e07ef09a9f882a89612ad555eca8f93
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7541
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
2022-12-21 13:09:29 +00:00