about summary refs log tree commit diff
path: root/tvix/eval/src/vm/generators.rs
AgeCommit message (Collapse)AuthorFilesLines
2024-08-09 r/8466 feat(tvix/eval): Implement builtins.readFileTypeIlan Joselevich1-0/+32
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-07-05 r/8343 refactor(tvix/eval): Drop LightSpan entirelyAspen Smith1-32/+32
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-05-30 r/8179 fix(tvix/eval/vm/generators): allow unused rq_{path_exists,read_dir}Florian Klink1-0/+2
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-04-13 r/7899 feat(tvix/eval): contextful JSON operationsRyan Lahfa1-3/+3
`toJSON` transform a Nix structure into a JSON string. For each context in that Nix structure, the JSON string must possess it. Thus, it is necessary to take the union of all contexts and attach it to the final structure. Unfortunately, the return type of `into_json` is a serde's JSON object, not a string. Therefore, it is not possible to reuse `NixString` machinery. Context tests are reinforced as Nix does not test those behaviors. Fixes b/393. Change-Id: I5afdbc4e18dd70469192c1aa657d1049ba330149 Signed-off-by: Ryan Lahfa <tvl@lahfa.xyz> Reviewed-on: https://cl.tvl.fyi/c/depot/+/11266 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-04-09 r/7882 fix(tvix): Avoid buffering file into memory in builtins.hashFileConnor Brewster1-11/+16
Right now `builtins.hashFile` always reads the entire file into memory before hashing, which is not ideal for large files. This replaces `read_to_string` with `open_file` which allows calculating the hash of the file without buffering it entirely into memory. Other callers can continue to buffer into memory if they choose, but they still use the `open_file` VM request and then call `read_to_string` or `read_to_end` on the `std::io::Reader`. Fixes b/380 Change-Id: Ifa1c8324bcee8f751604b0b449feab875c632fda Reviewed-on: https://cl.tvl.fyi/c/depot/+/11236 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2024-02-21 r/7591 feat(tvix/eval): Store string context alongside dataAspen Smith1-1/+1
Previously, Nix strings were represented as a Box (within Value) pointing to a tuple of an optional context, and another Box pointing to the actual string allocation itself. This is pretty inefficient, both in terms of memory usage (we use 48 whole bytes for a None context!) and in terms of the extra indirection required to get at the actual data. It was necessary, however, because with native Rust DSTs if we had something like `struct NixString(Option<NixContext>, BStr)` we could only pass around *fat* pointers to that value (with the length in the pointer) and that'd make Value need to be bigger (which is a waste of both memory and cache space, since that memory would be unused for all other Values). Instead, this commit implements *manual* allocation of a packed string representation, with the length *in the allocation* as a field past the context. This requires a big old pile of unsafe Rust, but the payoff is clear: hello outpath time: [882.18 ms 897.16 ms 911.23 ms] change: [-15.143% -13.819% -12.500%] (p = 0.00 < 0.05) Performance has improved. Fortunately this change can be localized entirely within value/string.rs, since we were abstracting things out nicely. Change-Id: Ibf56dd16c9c503884f64facbb7f0ac596463efb6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10852 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: aspen <root@gws.fyi>
2024-02-21 r/7587 fix(tvix/eval): allow reading non-UTF8 filesFlorian Klink1-1/+1
With our values using bstr now, we're not restricted to only reading files that contain valid UTF-8. Update our `read_to_string` function to `read_to_end` (named like `std::io::Read::read_to_end`), and have it return a Vec<u8>. Change-Id: I87f0291dc855a132689576559c891d66c30ddf2b Reviewed-on: https://cl.tvl.fyi/c/depot/+/11003 Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de> Reviewed-by: Pádraic Ó Mhuiris <patrick.morris.310@gmail.com> Reviewed-by: flokli <flokli@flokli.de>
2024-02-13 r/7508 feat(tvix/eval): Box Value::CatchableAspen Smith1-3/+3
This is now the only enum variant for Value that is larger than 8 bytes (it's 16 bytes), so boxing it (especially since it's not perf-critical) allows us to get the Value size down to only 16 bytes! Change-Id: I98598e2b762944448bef982e8ff7da6d6683c4aa Reviewed-on: https://cl.tvl.fyi/c/depot/+/10798 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz> Autosubmit: aspen <root@gws.fyi>
2024-02-10 r/7496 refactor(tvix/eval): Box the inside of Value::JsonAspen Smith1-1/+1
serde_json::Value is pretty large, and is contributing (albeit not exclusively) to the large size of the Value repr. Putting it in a box is *especially* cheap (since it's rarely used) and allows us to (eventually) cut down on the size of Value. Change-Id: I005a802d8527b639beb4e938e3320b11ffa1ef23 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10795 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: aspen <root@gws.fyi> Tested-by: BuildkiteCI
2024-02-02 r/7467 refactor(tvix/eval): Box Value::StringAspen Smith1-2/+2
NixString is *quite* large - like 80 bytes - because of the extra capacity value for BString and because of the context. We want to keep Value small since we're passing it around a lot, so let's box the NixString inside Value::String to save on some memory, and make cloning ostensibly a little cheaper Change-Id: I343c8b4e7f61dc3dcbbaba4382efb3b3e5bbabb2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10729 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2024-01-18 r/7407 refactor(tvix/eval): generalize EvalIO containerFlorian Klink1-1/+8
Don't restrict to a Box<dyn EvalIO>. There's still one or two places where we do restrict, this will be solved by b/262. Change-Id: Ic8d927d6ea81fa12d90b1e4352f35ffaafbd1adf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10639 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2024-01-14 r/7379 fix(tvix/eval): catchable-aware builtinsRyan Lahfa1-2/+6
A bunch of operations in Tvix are not aware of catchable values and does not propagate them. In the meantime, as we wait for a better solution, we just offer this commit for moving the needle. Change-Id: Ic3f0e1550126b0847b597dfc1402c35e0eeef469 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10473 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2024-01-03 r/7334 feat(tvix/eval): contextful string coercionRyan Lahfa1-1/+1
String with contexts are always coerced to a string with the same context. Change-Id: I224814febd9cad196bb28876793e76bed564dc72 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10440 Tested-by: BuildkiteCI Autosubmit: raitobezarius <tvl@lahfa.xyz> Reviewed-by: sterni <sternenseemann@systemli.org>
2023-12-29 r/7274 fix(tvix/eval): catchable in type field of nix_eq()Adam Joseph1-2/+3
Change-Id: I165ff77764e272cc94d18cb03ad6cbc9a8ebefde Reviewed-on: https://cl.tvl.fyi/c/depot/+/10348 Autosubmit: Adam Joseph <adam@westernsemico.com> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2023-12-14 r/7218 fix(tvix/eval): remove incorrect imports when coercingsterni1-4/+13
The default behavior of string coercion in C++ Nix is to weakly coerce and import to store if necessary. There is a flag to make it strongly coerce (coerceMore) and a flag that controls whether path values have the corresponding file/directory imported into the store before returning the (store) path as a string (copyToStore). We need to implement our equivalent to the copyToStore (import_paths) flag for the benefit of weak coercions that don't import into the store (dirOf, baseNameOf, readFile, ...) and strong coercions that don't import into the store (toString). This makes coerce_to_string as well as CoercionKind weirder and more versatile, but prevents us from reimplementing parts of the coercion logic constantly as can be seen in the case of baseNameOf. Note that it is not possible to test this properly in //tvix/eval tests due to the lack of an appropriate EvalIO implementation being available. Tests should be added to //tvix/glue down the line. Change-Id: I8fb8ab99c7fe08e311d2ba1c36960746bf22f566 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10361 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-12-12 r/7176 feat(tvix/eval): nonrecursive coerce_to_string()Adam Joseph1-2/+2
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 Joseph1-8/+8
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/7167 feat(tvix/eval): nonrecursive nix_cmp_ordering(), fixes b/339Adam Joseph1-1/+1
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/7164 feat(tvix/eval): nonrecursive nix_eq()Adam Joseph1-6/+6
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-11-05 r/6955 chore(tvix): fix trivial clippy lintsVincent Ambo1-1/+1
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-09-24 r/6650 fix(tvix/eval): fix b/281 by adding Value::CatchableAdam Joseph1-15/+13
This commit makes catchable errors a variant of Value. The main downside of this approach is that we lose the ability to use Rust's `?` syntax for propagating catchable errors. Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-07-22 r/6439 feat(tvix/store/proto): use Bytes instead of Vec<u8>Florian Klink1-2/+2
Makes use of https://github.com/tokio-rs/prost/pull/341, which makes our bytes field cheaper to clone. It's a bit annoying to configure due to https://github.com/hyperium/tonic/issues/908, but the workaround does get the job done. Change-Id: I25714600b041bb5432d3adf5859b151e72b12778 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8975 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: flokli <flokli@flokli.de>
2023-07-21 r/6436 refactor(tvix/store): use bytes for node names and symlink targetsFlorian Klink1-3/+2
Some paths might use names that are not valid UTF-8. We should be able to represent them. We don't actually need to touch the PathInfo structures, as they need to represent StorePaths, which come with their own harder restrictions, which can't encode non-UTF8 data. While this doesn't change any of the wire format of the gRPC messages, it does however change the interface of tvix_eval::EvalIO - its read_dir() method does now return a list of Vec<u8>, rather than SmolStr. Maybe this should be OsString instead? Change-Id: I821016d9a58ec441ee081b0b9f01c9240723af0b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8974 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-06-21 r/6341 fix(tvix/eval): use realpaths for import cachesterni1-1/+1
I've noticed this behavior when writing the admittedly cursed test case included in this CL. Alternatively we could use some sort of machinery using `builtins.trace`, but I don't think we capture stderr anywhere. I've elected to put this into the eval cache itself while C++ Nix does it in builtins.import already, namely via `realisePath`. We don't have an equivalent for this yet, since we don't support any kind of IfD, but we could revise that later. In any case, it seems good to encapsulate `ImportCache` in this way, as it'll also allow using file hashes as identifiers, for example. C++ Nix also does our equivalent of canon_path in `builtins.import` which we still don't, but I suspect it hardly makes a difference. Change-Id: I05004737ca2458a4c67359d9e7d9a2f2154a0a0f Reviewed-on: https://cl.tvl.fyi/c/depot/+/8839 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-06-11 r/6266 fix(tvix/eval): emit only warnings on shadowed outputsLinus Heckemann1-1/+1
Unfortunately, nixpkgs has at least one case[1] where the out environment variable is shadowed -- though it doesn't cause a problem, since it's shadowed with the correct value, odd as this may be! [1]: https://github.com/NixOS/nixpkgs/blob/c7c298471676ac1c7789ab3c424fbcebecaa6791/pkgs/development/python-modules/pybind11/default.nix#L19 Change-Id: Ibf6790d2484dc9cce8e424feeb5886664d498dc3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8696 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-05-22 r/6173 refactor(tvix/eval): use &Path instead of PathBufFlorian Klink1-3/+3
This allows getting rid of some clones in eval/src/vm/generators.rs. Change-Id: I330390307d3bcfeef19c98954c753ee55b1ccee3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8604 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-22 r/6171 refactor(tvix/eval/io): use io::Error instead of tvix_eval errorsFlorian Klink1-6/+29
We didn't return anything useful other than ErrorKind::IO anyways. We can use io::ErrorKind::Unsupported for DummyIO. Fixes b/271. Change-Id: Icb231e9b38168e8b6fa473bfa405d160357b317f Reviewed-on: https://cl.tvl.fyi/c/depot/+/8602 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-03-17 r/6025 feat(tvix/eval): track span of first force in a thunk blackholeVincent Ambo1-4/+12
This is step 1 towards being able to use all 4 spans that we know when dealing with infinite recursion. It tracks the span at which the force of a thunk was first requested when constructing a blackhole, so that we can highlight the spans of the first and second forces. These are actually the least relevant spans, but the easiest to put in place, more coming soon. Change-Id: I4c7e82f6211b98756439d4148a4191457cc46807 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8269 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-03-17 r/6023 feat(tvix/eval): enrich errors with VM's frame stack informationVincent Ambo1-15/+7
When emitting an error at runtime, the VM will now use the new `NativeError` and `BytecodeError` error kinds (which just wrap inner errors) to create a set of diagnostics to emit. The primary diagnostic is emitted last, with `error` type (so it will be coloured red in terminals), the other ones will be emitted with `note` type, highlighting the causal chain. Example: https://gist.github.com/tazjin/25feba7d211702453c9ebd5f8fd378e4 This is currently quite verbose, and we can cut down on this further, but the purpose of this commit is to surface more information first of all before worrying about the exact display. Change-Id: I058104a178c37031c0db6b4b3e4f4170cf76087d Reviewed-on: https://cl.tvl.fyi/c/depot/+/8266 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-03-14 r/5992 chore(tvix): Generator{Request|Response} -> VM{Request|Response}Vincent Ambo1-121/+112
We settled on this being the most reasonable name for this construct. Change-Id: Ic31c45461a842f22aa05f4446123fe3a61dfdbc0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8291 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5985 docs(tvix/eval): fix reference to `Empty` message in a commentVincent Ambo1-1/+1
Change-Id: I3dc30cca33fbbd8e8686655635ee471f5937d9f8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8257 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5984 refactor(tvix/eval): rename VM::tail_call_value -> VM::call_valueVincent Ambo1-1/+1
The name of this was not accurate anymore after all the recent shuffling, as noted by amjoseph. Conceptual tail calls here only occur for Nix bytecode calling Nix bytecode, but things like a builtin call actually push a new native frame. Change-Id: I1dea8c9663daf86482b8c7b5a23133254b5ca321 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8256 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5983 fix(tvix/eval): emit warnings from builtins.import againVincent Ambo1-7/+28
Wires up generator logic to emit warnings that already have spans attached again. Change-Id: I9f878cec3b9d4f6f7819e7c71bab7ae70bd3f08b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8224 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5979 fix(tvix/eval): implement cppnix JSON-serialisation semanticsVincent Ambo1-0/+23
This drops the usage of serde::Serialize, as the trait can not be used to implement the correct semantics (function colouring!). Instead, a manual JSON serialisation function is written which correctly handles toString, outPath and other similar weirdnesses. Unexpectedly, the eval-okay-tojson test from the C++ Nix test suite now passes, too. This fixes an issue where serialising data structures containing derivations to JSON would fail. Change-Id: I5c39e3d8356ee93a07eda481410f88610f6dd9f8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8209 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5978 feat(tvix/eval): give generators human-readable namesVincent Ambo1-19/+26
This adds static strings to generator frames that describe the generator in a human-readable fashion, which are then logged in observers. This makes runtime traces very precise, explaining exactly what is being requested from where. Change-Id: I695659a6bd0b7b0bdee75bc8049651f62b150e0c Reviewed-on: https://cl.tvl.fyi/c/depot/+/8206 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5974 refactor(tvix/eval): print only *types* when observing generatorsVincent Ambo1-7/+11
Do not print the entire value (they're likely to be thunks anyways). This is useful because there *can* be cases where something like `nixpkgs` itself is sent through one of these messages, in which case the observer trying to print it will just blow up. Change-Id: I1fa37ea071d75efa0eb3428c6e2fe4351c62be6b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8202 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-13 r/5964 refactor(tvix/eval): flatten call stack of VM using generatorsVincent Ambo1-51/+234
Warning: This is probably the biggest refactor in tvix-eval history, so far. This replaces all instances of trampolines and recursion during evaluation of the VM loop with generators. A generator is an asynchronous function that can be suspended to yield a message (in our case, vm::generators::GeneratorRequest) and receive a response (vm::generators::GeneratorResponsee). The `genawaiter` crate provides an interpreter for generators that can drive their execution and lets us move control flow between the VM and suspended generators. To do this, massive changes have occured basically everywhere in the code. On a high-level: 1. The VM is now organised around a frame stack. A frame is either a call frame (execution of Tvix bytecode) or a generator frame (a running or suspended generator). The VM has an outer loop that pops a frame off the frame stack, and then enters an inner loop either driving the execution of the bytecode or the execution of a generator. Both types of frames have several branches that can result in the frame re-enqueuing itself, and enqueuing some other work (in the form of a different frame) on top of itself. The VM will eventually resume the frame when everything "above" it has been suspended. In this way, the VM's new frame stack takes over much of the work that was previously achieved by recursion. 2. All methods previously taking a VM have been refactored into async functions that instead emit/receive generator messages for communication with the VM. Notably, this includes *all* builtins. This has had some other effects: - Some test have been removed or commented out, either because they tested code that was mostly already dead (nix_eq) or because they now require generator scaffolding which we do not have in place for tests (yet). - Because generator functions are technically async (though no async IO is involved), we lose the ability to use much of the Rust standard library e.g. in builtins. This has led to many algorithms being unrolled into iterative versions instead of iterator combinations, and things like sorting had to be implemented from scratch. - Many call sites that previously saw a `Result<..., ErrorKind>` bubble up now only see the result value, as the error handling is encapsulated within the generator loop. This reduces number of places inside of builtin implementations where error context can be attached to calls that can fail. Currently what we gain in this tradeoff is significantly more detailed span information (which we still need to bubble up, this commit does not change the error display). We'll need to do some analysis later of how useful the errors turn out to be and potentially introduce some methods for attaching context to a generator frame again. This change is very difficult to do in stages, as it is very much an "all or nothing" change that affects huge parts of the codebase. I've tried to isolate changes that can be isolated into the parent CLs of this one, but this change is still quite difficult to wrap one's mind and I'm available to discuss it and explain things to any reviewer. Fixes: b/238, b/237, b/251 and potentially others. Change-Id: I39244163ff5bbecd169fe7b274df19262b515699 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8104 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2023-03-13 r/5963 feat(tvix/eval): implement generator-based Nix equality logicVincent Ambo1-5/+1
Adds a `Value::neo_nix_eq` method (the `neo_` prefix will be dropped when we flip over to the generator implementation of the VM) which implements Nix equality semantics using async, generator-based comparisons. Instead of tracking the "kind" of equality that is being compared (see the pointer-equality doc) through a pair of booleans, I've introduced an enum that explicitly lists the possible comparisons. Change-Id: I3354cc1470eeccb3000a5ae24f2418db1a7a2edc Reviewed-on: https://cl.tvl.fyi/c/depot/+/8241 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
2023-03-07 r/5899 feat(tvix/eval): introduce generators moduleVincent Ambo1-0/+538
This module contains the request/response types for generators requesting actions from the VM. For most of these, an async helper function is added that will be used inside of generator functions to make use of these requests/responses instead of constructing them directly. Change-Id: I1e085f88adaf784a34867957a0e82532d3a83d7c Reviewed-on: https://cl.tvl.fyi/c/depot/+/8148 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>