about summary refs log tree commit diff
path: root/tvix (follow)
AgeCommit message (Collapse)AuthorFilesLines
2023-12-12 r/7203 fix(tvix/eval): builtins.length: propagate catchablesAdam Joseph3-0/+5
This commit fixes out builtins.length so it propagates catchables like cppnix does. Change-Id: I7670bec5eee1d4cd3f67a04c9a6808979fb56a8d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10315 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7201 fix(tvix/eval): builtins.filter: propagate catchablesAdam Joseph3-2/+7
This commit fixes builtins.filter so it propagates catchables correctly. Change-Id: Ib23a383bc5e272e42052205ffd1e94649a0ebc47 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10313 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7200 feat(tvix/eval): builtins.hashString: add placeholderAdam Joseph1-0/+12
This adds an unimplemented placeholder for builtins.hashString. Change-Id: Ibc770103acf5dbc3ea7589ab5ca23fe6e07bd91a Reviewed-on: https://cl.tvl.fyi/c/depot/+/10311 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7199 fix(tvix/eval): builtins.getAttr: propagate catchablesAdam Joseph3-0/+8
Change-Id: I84b6b8f8568d57614a03aff0d6069e0bc27357bf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10310 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7198 fix(tvix/eval): builtins.elemAt: propagate catchablesAdam Joseph3-0/+8
This commit fixes builtins.elemAt so it propagates catchables like cppnix does. Change-Id: Ieca5e128da17e78af0b14dae4a28a1ff8796e4f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10308 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7197 feat(tvix/glue): make unimplemented-structuredAttrs catchableAdam Joseph1-1/+3
This commit adjusts the error produced by STRUCTURED_ATTRS so that it is catchable. This way we are able to enumerate the release packageset, and the enumeration process will simply treat the few derivations using structured attributes as being broken, rather than killing the whole eval session. Change-Id: I2e17638b8e3227f88543c3718aaf505deaec22ae Reviewed-on: https://cl.tvl.fyi/c/depot/+/10306 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7196 fix(tvix/eval): propagate catchables through builtins.splitVersionAdam Joseph3-0/+5
This fixes our implementation of builtins.splitVersion so it propagates catchables like cppnix does. Change-Id: Id5d83ea76229f8c8f202aa42353cb609e67de43f Reviewed-on: https://cl.tvl.fyi/c/depot/+/10305 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7195 feat(tvix/eval): builtins.hasContext: placeholder implementationAdam Joseph1-0/+8
Currently this just `throw`s a message explaining that it is not implemented. This is necessary in order to allow enumerating the nixpkgs release attrset (afaict only one package uses this builtin). Change-Id: I45266d46af579ddb5856b192b6be4b481369543c Reviewed-on: https://cl.tvl.fyi/c/depot/+/10302 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7194 fix(tvix/eval): baseNameOf should not coerce paths into stringsAdam Joseph1-4/+12
... since this may import them to the store which changes their basename. Fixes b/350. Change-Id: Iabd08ff4d6a424c66d6d7784d7a96b0c078f0a91 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10298 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7193 fix(tvix/eval): add unimplemented __curPos and builtins.filterSourceAdam Joseph3-0/+19
This commit adds __curPos (to the global scope, yuck) and builtins.filterSource. These are not implemented; forcing them will produce the same result as `throw "message"`. Unfortunately these two post-2.3 features are used throughout nixpkgs. Since an unresolved indentifier is a catchable error, this breaks the entire release eval. With this commit, it simply causes those broken packages that use these features to appear as they are: broken. Change-Id: Ib43dea571f6a9fab4d54869349f80ee4ec5424c2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10297 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7192 fix(tvix/eval): propagate catchables through `&&`Adam Joseph3-0/+4
Change-Id: I7bb5ac1ef47b41c47269e64cee0e90eb64c6fcce Reviewed-on: https://cl.tvl.fyi/c/depot/+/10322 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7191 fix(tvix/eval): make `||` propagate catchablesAdam Joseph3-0/+4
Change-Id: I42f994d7c9228368d5f6c30c4730c24666f7bc69 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10320 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7190 fix(tvix/eval): fix nested assertions b/340Adam Joseph3-0/+2
This commit fixes our handling of `throw` within an `assert` condition. Fixes: b/340 Change-Id: I40a383639ec266da50a853f16216b1b7868495da Reviewed-on: https://cl.tvl.fyi/c/depot/+/10318 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7189 fix(tvix/eval): `?`: propagate catchablesAdam Joseph3-8/+18
This commit fixes out `?` operator so it correctly propagates catchables. Change-Id: Iebaa153a8492101ee3ddd29893c98730ff331547 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10317 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7188 feat(tvix/eval): OpAttrsSelect should propagate catchablesAdam Joseph1-11/+19
Previously, using a catchable as either argument of OpAttrsSelect would result in an unrecoverable error. This commit matches cppnix behavior by propagating the catchable. Change-Id: I4877f4068ec2b823225f185290693c101d0b9c9e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10303 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7187 fix(tvix/eval): calling a catchable is catchableAdam Joseph3-0/+8
When attempting to call a Value, if it is a Value::Catchable we must not cause an uncatchable failure. This commit simply reuses the Value::Catchable as the result of attempting to call it. This is safe because nix is designed so that nix code cannot distinguish between different catchable failures -- they all look the same to the interpreted code. This fixes b/351. Change-Id: Ibf763a08753e541843626182ff59fdbf15ea2959 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10300 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7186 fix(tvix/eval): fix catchables in named formalsAdam Joseph3-11/+27
Fixes b/348. Change-Id: I5e8d56b5fd26a19eac32ec5e11baf93765691dc8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10296 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7185 test(tvix/eval): test catchables in named formalsAdam Joseph2-0/+2
Relates to b/348. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (({ fred }: "bob") (throw "3"))).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this Nix code --> [code]:1:20 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'set', but found a 'internal[catchable]' --> [code]:1:21 | 1 | (builtins.tryEval (({ fred }: "bob") (throw "3"))).success | ^^^^^^^^ Change-Id: I730fdd996f7e1b81dbbf83dc1524104a8cad2f78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10295 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7184 fix(tvix/eval): fix testing catchables for inequalityAdam Joseph3-2/+7
Fixes b/347. Change-Id: Icad0251884d4d8adcdf8d690b91385bf4896f9c8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10294 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7183 test(tvix/eval): testing catchable for inequalityAdam Joseph2-0/+2
Relates to b/347. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (throw "bob" != 3)).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'bool', but found a 'internal[catchable]' --> [code]:1:20 | 1 | (builtins.tryEval (throw "bob" != 3)).success | ^^^^^^^^^^^^^^^^ Change-Id: Ide19b3ddaf314ef310efc2fe5ac36667e43011dc Reviewed-on: https://cl.tvl.fyi/c/depot/+/10293 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7182 fix(tvix/eval): handle catchables in attribute set updatesAdam Joseph3-4/+11
Fixes b/346. Change-Id: I277121d2363e605ebe09651ed9440fe1bc126c8c Reviewed-on: https://cl.tvl.fyi/c/depot/+/10292 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7181 test(tvix/eval): test for catchable in attribute mergesAdam Joseph2-0/+2
Relates to b/346. $ /git/depot/result/bin/tvix -E '(builtins.tryEval (throw "bob" // { })).success' note: while evaluating this Nix code --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (force) --> [code]:1:1 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: while evaluating this as native code (tryEval) --> [code]:1:2 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E006]: expected value of type 'set', but found a 'internal[catchable]' --> [code]:1:20 | 1 | (builtins.tryEval (throw "bob" // { })).success | ^^^^^^^^^^^^^^^^^^ Change-Id: Ib84c4ec6d2612d4f1f6066e66c3dd1bf04369b6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10291 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7180 fix(tvix/eval): fix recovering from throws in implicationsAdam Joseph3-0/+2
This fixes b/345. Change-Id: Ic0d3b6ffacd2a5e0050d22354d08320b69a4fe13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10290 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7179 test(tvix/eval): test recovering from throw in implicationsAdam Joseph2-0/+2
error[E006]: expected value of type 'bool', but found a 'internal[catchable]' --> src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-implications.nix:1:43 | 1 | (builtins.tryEval (({ foo ? throw "up" }: foo -> true) { })).success | ^^^^^^^^^^^ Relates to b/345 Change-Id: Ic331c32ea59bf67ae775f485b444dc6804ca13d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10289 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7178 fix(tvix/eval): fix branching on catchable defaults (b/343)Adam Joseph6-0/+20
This commit adds Opcode::OpJumpIfCatchable, which can be inserted ahead of most VM operations which expect a boolean on the stack, in order to handle catchables in branching position properly. Other than remembering to patch the jump, no other changes should be required. This commit also fixes b/343 by emitting this new opcode when compiling if-then-else. There are probably other places where we need to do the same thing. Change-Id: I48de3010014c0bbeba15d34fc0d4800e0bb5a1ef Reviewed-on: https://cl.tvl.fyi/c/depot/+/10288 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7177 test(tvix/eval): test branching on catchable defaults (b/343)Adam Joseph2-0/+2
This is a test case for b/343, wherein tvix dies if you try to branch on an argument whose defaulted value is a catchable. Change-Id: I891ca825e39ad14dda9f220f06d9591874fcd45d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10287 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7176 feat(tvix/eval): nonrecursive coerce_to_string()Adam Joseph6-121/+138
After this commit, the only non-builtins uses of generators are: - coerce_to_string() uses generators::request_enter_lambda() - Thunk::force() uses generators::request_enter_lambda() That's it! Once those two are taken care of, GenCo can become an implementation detail of `builtins::BuiltinGen`. No more crazy nonlocal flow control within the interpreter: if you've got a GenCo floating around in your code it's because you're writing a builtin, which isn't part of the core interpreter. The interpreter won't need GenCos to talk to itself anymore. Technically generators::request_path_import() is also used by coerce_to_string(), but that's just because the io_handle happens to be part of the VM. There's no recursion-depth issue there, so the call doesn't need to go through the generator mechanism (request_path_import() doesn't call back to the interpreter!) Change-Id: I83ce5774d49b88fdafdd61160975b4937a435bb0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10256 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7175 feat(tvix/eval): nonrecursive deep_force()Adam Joseph5-80/+82
This commit implements deep_force() nonrecursively, by maintaining an explicit stack rather than using the call stack for recursion. As an added bonus, we don't need to pass around the SharedThunkSet anymore, and can in fact completely eliminate SharedThunkSet. Change-Id: I7c4f59f37834d451a28bf6be317eb0a90eac4ee6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10252 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7174 docs(tvix/eval): clarify difference between ThunkSet and BlackholeAdam Joseph1-2/+5
The comment explaining ThunkSet makes it seem like it does the same think as ThunkRepr::Blackhole. In fact neither one is a substitute for the other. Let's explain the difference. Change-Id: I89ceaaa9d3c499edbc7d48f70ca5d11f97666c43 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10250 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7173 test(tvix/eval): nested assertions (b/340)Adam Joseph2-0/+2
Change-Id: I898d7056877a6370d5720b633df416f54e7cf65f Reviewed-on: https://cl.tvl.fyi/c/depot/+/10222 Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7172 feat(tvix/eval): drop LightSpan::DelayedAdam Joseph2-13/+1
LightSpan::Delayed was introduced in commit bf286a54bc2ac5eeb78c3d5c5ae66e9af24d74d4 which claimed that "This reduces the eval time for `builtins.length (builtins.attrNames (import <nixpkgs> {}))` by *one third*!" I am unable to reproduce this result. In fact, dropping the LightSpan::Delayed variant of the enum makes eval of the same expression slightly faster! I also tried a large evaluation (pkgsCross...hello) and got similar results: slightly faster, slightly less memory. See git footers. I suspect that there was some unrelated horrific inefficiency that has since been fixed. The avoided computation in `get_span()` is nothing more than a binary search! If this were in fact a major performance issue we could simply precompute the mapping from CodeIdx to Span when the Chunk becomes immutable (i.e. at the end of the compilation process, when compiler backtracking is no longer a concern). Since a Span is just 64 bits this is not a space issue, and since binary search is much simpler than compiling Nix expressions it isn't a performance issue either. Technically there is no longer any reason to have LightSpan since it is now a single-variant enum. However there is no rush to remove it, since Rust will optimize its representation into the same thing you'd get if you replaced LightSpan by Span. Prev-Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}} This-Benchmark: {"nixpkgs-attrnames":{"kbytes":"230192","system":"0.29","user":"2.00"}} Prev-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"458936","system":"0.73","user":"5.36"}} This-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"451808","system":"0.53","user":"5.10"}} Change-Id: Ib9e04806850aa1fc4e66e2a042703986440a7b4e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10254 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-12 r/7171 feat(tvix/cli): add benchmark for bf286a54bcAdam Joseph1-0/+2
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 r/7170 refactor(tvix): use io::Result for EvalIOFlorian Klink3-22/+22
This is just a alias for Result<_, io::Error>, but shorter. Change-Id: I7c22f61b85e3014885a747b5c1e5abd11b0ef17d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10327 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-12-12 r/7169 fix(tvix/eval): preserve catchables in nix_cmp_ordering(), fix b/338Adam Joseph5-9/+15
This commit fixes b/338 by properly propagating catchables through comparison operations. Change-Id: I6b0283a40f228ecf9a6398d24c060bdacb1077cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10221 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7168 test(tvix/eval): test for b/338 catchable hygiene problemAdam Joseph2-0/+2
Commit 05f42519b53575ad3235b5e0a0cd7d71f04076a5 fixed b/281 by establishing a hygiene regimen to partition *catchable* errors (i.e. those which tryEval can detect) from all other errors, like internal VM failures or I/O errors (which Nix must not be allowed to detect, since these errors are fundamentally impure). Unfotunately there are still cases where tvix assumes that anything other than Value::Bool means it should panic!(). I found another one, and added a test case for it in: eval_okay_src_tests_tvix_tests_eval_okay_compare_ordering_catchable_nix Not yet passing. Change-Id: I69c62ed9ea5c8f81870e8de5c5fe12dcde849763 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10220 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7167 feat(tvix/eval): nonrecursive nix_cmp_ordering(), fixes b/339Adam Joseph7-44/+75
This commit rewrites Value::nix_cmp_ordering() into an equivalent nonrecursive form. Except for calls to Thunk::force(), the new form no longer uses generators, and is async only because of the fact that it calls Thunk::force(). I originally believed that this commit would make evaluation faster. In fact it is slightly slower. I believe this is due to the added vec![] allocation. I am investigating. Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"} This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460224","system-seconds":"0.67","user-seconds":"5.84"} Change-Id: Ic627bc220d9c5aa3c5e68b9b8bf199837cd55af5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10212 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7166 fix(tvix/eval): never use partial_cmp() (partial fix b/338)Adam Joseph3-23/+16
This is part of a fix for b/338. We should never use PartialOrd::partial_cmp(). All Nix types except floats are obviously totally-ordered. In addition, it turns out that because Nix treats division by zero rather than producing a NaN, and because it does not support "negative zero", even floats are in fact totally ordered in Nix. Therefore, every call to PartialOrd::partial_cmp() in tvix is an error. We have to *implement* this function, but we should never call it on built-in types. Moreover, nix_cmp_ordering() currently returns an Option<Ordering>. I'm not sure what was going on there, since it's impossible for it to return None. This commit fixes it to return simply Ordering rather than Option<Ordering>. Change-Id: If5c084164cf19cfb38c5a15554c0422faa5f895d Reviewed-on: https://cl.tvl.fyi/c/depot/+/10218 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-12 r/7165 test(tvix/eval): add test case for b/339Adam Joseph3-0/+3
Not yet passing. Change-Id: I1de3f72d8b3f46567fdba010fc3ab4bace3f1699 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10219 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-12-12 r/7164 feat(tvix/eval): nonrecursive nix_eq()Adam Joseph4-137/+159
This commit rewrites Value::nix_eq() into an equivalent. Except for calls to Thunk::force(), the new form no longer uses generators, and is async only because of the fact that it calls Thunk::force(). I believed that the nonrecursive form would be faster. It is, in fact, slightly slower. I believe this is due to the vec![] allocation; I am investigating. Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"459068","system-seconds":"0.71","user-seconds":"5.39"} This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"} Change-Id: I10f4868891e4b7475df13f0cbc41ec78dd985dd8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10118 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7163 feat(tvix/cli): add macrobenchmarkAdam Joseph1-1/+61
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 r/7161 feat(tvix/glue): add nixpkgs eval benchmarkFlorian Klink5-0/+88
This adds a criterion.rs-based testbench into tvix-glue. It can be invoked by running `cargo bench` from inside the `tvix-glue` crate. `target/criterion/report/index.html` contains nice graphs. It's able to diff against the previous run, so you can invoke `cargo bench` before and after a certain change to reason about the impact in evaluation performance. Currently, we need to create a bunch of Evaluator resources inside the benchmark loop itself, which is a bit annoying, as it leaks into the things we benchmark. This should become better with b/262. Fixes b/322. Change-Id: I91656a308887baa1d459ed54d58baae919a4aaf2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10245 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-12 r/7160 chore(tvix/glue): allow unused_variablesFlorian Klink1-0/+1
cl/9364 did introduce a warning here, which is visible when building in release mode - or invoking `cargo bench` in tvix-glue. Change-Id: Ia82082a58543f0fdd32866fdfcd37d0a5fdfda9c Reviewed-on: https://cl.tvl.fyi/c/depot/+/10261 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-12-11 r/7159 refactor(tvix/*store/sled): make ::new() more genericFlorian Klink6-11/+10
We don't really require the Path to be a PathBuf, we don't even require it to be a Path, we only need it to be AsRef<Path>>. This removes some conversion in the from_addr cases, which can just reuse `url.path()` (a `&str`). Change-Id: I38d536dbaf0b44421e41f211a9ad2b13605179e9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10258 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-12-11 r/7158 chore(tvix): drop sled compression supportFlorian Klink9-16/+14
It's been a while since the last sled release, and that one binds to a pretty old version of zstd, requiring workarounds like cl/10090. Upstream sled main branch currently has zstd halfway patched out (it's a no-op, but the feature flag and options are still there), and it's in that state for a year. Rather than maintaining our own fork of sled, let's just stop using the compression feature in sled, dropping the version pin to zstd that way, removing the need for cl/10090. This doesn't mean we won't reintroduce per-blob compression - but we probably just won't let sled take care of the compression, but do it ourselves - which is necessary for more chunked blob storage anyways. Even though we do drop the feature flag, we still need to explicitly use use_compression(false). Change-Id: I0e4892d29e41c76653272dc1a3625180da6fee12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10257 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-12-11 r/7157 chore(tvix/proto): drop evaluator.protoFlorian Klink2-165/+0
This only contained the (unused) evaluator.proto file. Considering we're less likely to have the CLI talk to a long-running evaluator, but instead embed the evaluator inside the CLI, remove this. If we add a RPC to speak to an evaluator, we can resurrect this from git history. Change-Id: I2196aade55221660330dfd32dc3e52c39ec6ed43 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10241 Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-11 r/7156 refactor(tvix): use granular proto filesFlorian Klink2-23/+17
Only pass in the proto files that are actually needed to build that crate. They are already constructed in depot.tvix.$crate.protos.protos. Change-Id: If4381e6c3350e420ee4ddce1e0513bfe970678a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10240 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-11 r/7155 refactor(tvix/*/protos): separate lint targetFlorian Klink3-46/+107
Break up the go-bindings derivation. Keep "protos" containing all proto files (well, and the buf config), and use it for a check phase running linter and formatter, as well as the existing "go-bindings" attribute Change-Id: I52cb9d08570bb76452acb831eb711c5b6c0eacfb Reviewed-on: https://cl.tvl.fyi/c/depot/+/10239 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-11 r/7154 feat(tvix/build): add derivation_to_build_requestFlorian Klink7-0/+272
This function converts from a nix_compat::derivation::Derivation to a BuildRequest. In addition to the Derivation itself, it needs two lookup functions to map input paths to their castore nodes. Change-Id: I0332982f0bc7933a5fda137fe39d5a850639d929 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10236 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-12-11 r/7153 docs(tvix/*store-go): fix README invocationFlorian Klink2-3/+3
The command is called `regenerate`, not `generate`. Change-Id: I18075042ebd461e4dd0718a936e6bbe738a144d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10259 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
2023-12-11 r/7152 feat(tvix/build-go): initFlorian Klink8-0/+869
This adds the generated golang bindings for tvix-build. Change-Id: I2eb0d1cc38bc2fa34afd7c904eea05c5ee192cce Reviewed-on: https://cl.tvl.fyi/c/depot/+/10242 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
generated by cgit-pink 1.4.1 (git 2.44.1) at 2024-06-29T06·04+0000