about summary refs log tree commit diff
path: root/tvix/eval/src
AgeCommit message (Collapse)AuthorFilesLines
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/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/7170 refactor(tvix): use io::Result for EvalIOFlorian Klink1-12/+12
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 Joseph2-0/+2
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-09 r/7134 refactor(tvix/eval): address clippy lintsFlorian Klink1-1/+1
Change-Id: Ic2bd4e8291b30ceac9fa0e88a4f56e61ae99b603 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10227 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-12-09 r/7130 feat(tvix/eval): impl DoubleEndedIter for OwnedAttrsIteratorAdam Joseph1-0/+10
Change-Id: I4bd85dbe9c27047f4abbdeff4e2b796e9bcab3a1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10211 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-12-06 r/7120 feat(tvix/eval): rewrite Thunk::force() in nonrecursive formAdam Joseph3-64/+97
This commit rewrites Thunk::force() so that it is not (directly) self-recursive. It maintains a Vec of all the previously-encountered thunks which point to the one it is currently forcing, rather than recursively calling itself. Benefits: - Short term: This commit saves the cost of a round-trip through the generator machinery for the generators::request_force() which is removed by this commit. - Medium term: Once a similar transformation has been applied to nix_cmp(), nix_add(), nix_eq(), and coerce_to_string(), those four functions, along with Thunk::force(), will make non-tail calls only to each other. They can then be merged into a single tail-recursive function which does not use the generator machinery at all: enum Task { Cmp, Add, Eq, CoerceToString, Force}; fn Value::walk(task:Task, v1:Value, v2:Value) { // ... - Long term: The long-term goal here is to use generators **only for builtins** and [Marionette]-style remote control of the VM. In other words: use `async` for things that actually involve concurrency. Calls from the VM to builtins can then be blocking calls, because even cppnix will overflow the stack if you make a MAX_STACK_DEPTH-deep recursive call which passes through a builtin at every stack frame (e.g. `{ func = builtins.sort (a: b: ... func ...) ...}`). This way the inner "tight loop" of the interpreter doesn't pay the costs of `async` and generators. These costs manifest in terms of: performance, complex nonlocal control flow, and language impediments (async Rust is a restricted subset of real Rust, and is missing things like traits). [Marionette]: https://firefox-source-docs.mozilla.org/testing/marionette/Intro.html Change-Id: I6179b8abb2ea0492180fcb347f37595a14665777 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10039 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-12-05 r/7119 fix(tvix/eval): Return error rather than panicking on bad substringAspen Smith2-1/+10
If builtins.substring is invoked with (byte!!) offsets that aren't at codepoint boundaries, return an error rather than panicking. This is still incorrect (see b/337) but pushes the incorrectness forward a step. Change-Id: I5a4261f2ff250874cd36489ef598dcf886669d04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10199 Tested-by: BuildkiteCI Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org>
2023-11-25 r/7065 refactor(tvix/eval): use `or_default` helper in entry APIVincent Ambo1-3/+1
This fixes a future clippy lint. Change-Id: Ic830e94ef23595580c1037f10878c76bbb546dd9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10110 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-11-25 r/7064 fix(tvix): ensure PartialOrd/Ord agree for StorePath & NixStringVincent Ambo1-1/+1
This fixes a *future* clippy lint: https://rust-lang.github.io/rust-clippy/master/index.html#/incorrect_partial_ord_impl_on_ord_type In essence, because the implementation of *both* Ord and PartialOrd implies that ordering is not partial, all results of PartialOrd should simply be those of Ord. This is to avoid subtle bugs in future refactorings. Change-Id: I8fc6694010208752dd47746a2aaaeca0c788d574 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10109 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-11-25 r/7055 refactor(tvix/eval): add ThunkRepr::is_forced()Adam Joseph1-5/+9
Change-Id: I4eab5c81fb82337da06327248845cd2f3a4490d3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10038 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com>
2023-11-25 r/7054 feat(tvix/eval): add Thunk::unwrap_or_clone()Adam Joseph1-1/+34
This commit adds Thunk::unwrap_or_clone(), which uses Rc::try_unwrap() to avoid cloning the Value out of a an Rc which has only one strong reference. Change-Id: Icacefe0c823dcddf046d90c0c5cd5ed59fe976d4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10037 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-11-06 r/6963 docs(tvix/eval): document where EvalIO methods are usedVincent Ambo1-7/+34
Change-Id: I335f2ba4420973861c2a22125995b45a34d3608c Reviewed-on: https://cl.tvl.fyi/c/depot/+/9969 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 r/6958 chore(tvix): add missing clippy attributes & configVincent Ambo2-0/+2
For cases where clippy lints don't apply to us, or something is misfiring, add appropriate configuration. Relates to b/321. Change-Id: I0af453910b4a4112bf685b2a8e9a73de10ec87ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/9965 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-11-05 r/6957 refactor(tvix/eval): use IntoIterator trait for owned NixAttrs iterVincent Ambo1-15/+22
Uses the standard library IntoIterator trait for the construction of our iterators. Clippy complains about duplicating this. While doing this, I opted to rename the `IntoIter` type into something that is more useful to users, in case somebody ends up working with these manually. This fixes a clippy lint, and is related to b/321. Change-Id: I851fde0d7b8b38d182343a0fd6d9f8dd2a33ee11 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9963 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su>
2023-11-05 r/6955 chore(tvix): fix trivial clippy lintsVincent Ambo9-26/+25
Relates to b/321. Change-Id: I37284f89b186e469eb432e2bbedb37aa125a6ad4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9961 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su>
2023-11-03 r/6932 refactor(tvix/eval): delay allocation when comparing attr valuesVincent Ambo1-4/+4
Delays allocation (through cloning) of the values to be compared until *after* the keys have been compared. Change-Id: I7d68c27d7a0fbcdcc387db7c092bce50ca4b94ea Reviewed-on: https://cl.tvl.fyi/c/depot/+/9900 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-11-03 r/6931 chore(tvix/eval): add a marker for sorted borrowed attrs iterationVincent Ambo1-0/+6
Similar to `into_iter_sorted`, add a marker function for call sites that want *borrowed* sorted iteration. Change-Id: I7c6f14e1ac43fdb14b861b3da183eb5d12bba139 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9899 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-11-03 r/6930 refactor(tvix/eval): more efficiently intersect attributesVincent Ambo1-9/+70
builtins.intersectAttrs is used a _lot_ in nixpkgs eval, for whatever reason. We previously had a very inefficient implementation that would allocate for each comparison. It stuck out like a sore thumb in perf analysis. This moves to a custom algorithm with two iterators, one for the left and one for the right side, advancing them along the (borrowed) map keys until a match is found and allocation is required. I've not made any effort to reduce the verbosity of this code, I don't think it's worth it. On my machine this reduces the mean runtime of evaluating `nixpkgs.emacs.outPath` by ~8%. Change-Id: Ie506d82cb8d5f45909628f771a6b73e0eca16b27 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9898 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-09-26 r/6662 feat(tvix/eval): test case for b/281Adam Joseph2-0/+2
This commit adds a test case for b/281. Change-Id: I8dfbfc0ff636184d7882530d8aefb329a3af9e5c Reviewed-on: https://cl.tvl.fyi/c/depot/+/9288 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: flokli <flokli@flokli.de>
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-14T18·51+0000