about summary refs log tree commit diff
path: root/tvix/eval/src/builtins
AgeCommit message (Collapse)AuthorFilesLines
2023-05-26 r/6207 fix(tvix): don't call function eagerly in genList, map & mapAttrssterni1-5/+25
mapAttrs, map and genList call Nix functions provided by the caller and store the result of applying them in a Nix data structure that does not force all of its contents when forced itself. This means that when such a builtin application is forced, the Nix function calls performed by the builtin should not be forced: They may be forced later, but it is also possible that they will never be forced, e.g. in builtins.length (builtins.map (builtins.add 2) [ 1 2 3 ]) it is not necessary to compute a single application of builtins.add. Since request_call_with immediately performs the function call requested, Tvix would compute function applications unnecessarily before this change. Because this was not followed by a request_force, the impact of this was relatively low in Nix code (most functions return a new thunk after being applied), but it was enough to cause a lot of bogus builtins.trace applications when evaluating anything from `lib.modules`. The newly added test includes many cases where Tvix previously incorrectly applied a builtin, breaking a working expression. To fix this we add a new helper to construct a Thunk performing a function application at runtime from a function and argument given as `Value`s. This mimics the compiler's compile_apply(), but does itself not require a compiler, since the necessary Lambda can be constructed independently. I also looked into other builtins that call a Nix function to verify that they don't exhibit such a problem: - Many builtins immediately use the resulting value in a way that makes it necessary to compute all the function calls they do as soon as the outer builtin application is forced: * all * any * filter * groupBy * partition - concatMap needs to (shallowly) force the returned list for concatenation. - foldl' is strict in the application of `op` (I added a comment that makes this explicit). - genericClosure needs to (shallowly) force the resulting list and some keys of the attribute sets inside. Resolves b/272. Change-Id: I1fa53f744bcffc035da84c1f97ed25d146830446 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8651 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-05-12 r/6135 fix(tvix/eval): builtins.trace prints to stderrVincent Ambo1-1/+1
Change-Id: Icf577396035474d6977e627058aba5805c61985e Reviewed-on: https://cl.tvl.fyi/c/depot/+/8563 Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de>
2023-03-17 r/6022 fix(tvix/eval): use coerce_to_string in builtins.substringVincent Ambo1-1/+1
This actually uses coercion under the hood in C++ Nix. See the test for an example. Change-Id: Id56b364acf269225b6829d0b600e0222f8b3608d Reviewed-on: https://cl.tvl.fyi/c/depot/+/8322 Reviewed-by: andi <andi@notmuch.email> Tested-by: BuildkiteCI
2023-03-17 r/6020 chore(tvix/eval): remove some dead codeVincent Ambo1-31/+0
This was commented out and forgotten during the generator refactor, oh well. Change-Id: I474b685159a955a846db462da0dd0067af177b04 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8321 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-03-13 r/5983 fix(tvix/eval): emit warnings from builtins.import againVincent Ambo1-3/+4
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 Ambo2-31/+4
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/5977 fix(tvix/eval): handle toJSON on attribute sets with `outPath`Vincent Ambo1-6/+23
These are serialised as the serialisation of the value of that field. Change-Id: Ida51708b1f43ce09b0ec835f4e265918aa31dd09 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8205 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5976 fix(tvix/eval): handle `__toString` when JSON-serialising attrsetsVincent Ambo1-0/+9
These must be serialised to a JSON string of the *result* of coercing the function application to a string. Change-Id: Ib7f49ccd950503ddbdbf99643cd59565e26b50da Reviewed-on: https://cl.tvl.fyi/c/depot/+/8204 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5969 refactor(tvix/eval): box PathBufVincent Ambo1-3/+3
This shaves another 8 bytes off Value. How did that type get so big?! Change-Id: I65e9b59a1636bd57e3cc4aec5fea16887070b832 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8153 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5968 chore(tvix/eval): remove `From<SmolStr> for NixString` instanceVincent Ambo1-5/+4
No longer needed, and in some cases caused some extra work. Change-Id: I64e8e7292573bdc92a9c7a8e470e33f8c526f311 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8152 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5966 refactor(tvix/eval): wrap NixList in RcVincent Ambo1-3/+3
The size of a `Vector<Value>` is 64 *bytes*, which is quite large, and it bloated the entire Value type to this size. This change adds an indirection for the inner vector through Rc. Initially I tried to use a Box, but this breaks pointer equality guarantees for the Vector when it is small enough to be inlined. This reduces the size of Value from 64 to 32 bytes. Change-Id: Ic3211e861b1966c78b2c3d536ba291fea92647fd Reviewed-on: https://cl.tvl.fyi/c/depot/+/8150 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-13 r/5964 refactor(tvix/eval): flatten call stack of VM using generatorsVincent Ambo2-312/+328
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-04 r/5888 refactor(tvix/eval): remove VM argument from suspended native thunksVincent Ambo1-7/+1
Because they do not use it, and it can not be passed with the coming generator refactoring. Change-Id: I0d96f2357a7ee79cd8a0f401583d4286230d4a6b Reviewed-on: https://cl.tvl.fyi/c/depot/+/8146 Tested-by: BuildkiteCI Reviewed-by: raitobezarius <tvl@lahfa.xyz>
2023-03-04 r/5887 refactor(tvix/eval): insert storeDir "builtin" in eval startupVincent Ambo1-10/+0
Instead of using a suspended native thunk, calculate and optionally insert the storeDir builtin when the VM is constructed. We already have the IO handle available at this point and can just check whether a storeDir is present, and insert its absolute value as a builtin. Change-Id: If966eee6ff26dc888b6e888e7c46170c0c346b05 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8145 Reviewed-by: raitobezarius <tvl@lahfa.xyz> Tested-by: BuildkiteCI
2023-03-03 r/5870 chore(tvix/eval): fix clippy warningsVincent Ambo1-2/+2
Change-Id: I4c02f0104c455ac00a3f299c1fbf75cbb08e8972 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8142 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-02-13 r/5850 chore(tvix/eval): use writeln for newline stringAaqa Ishtyaq1-2/+2
This CL address clippy warning which expects to use `writeln` instead of `write` for strings with new line. Change-Id: Ia72a07502c60cfd489ecf1e3833b9d42d44a8b17 Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/8030 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2023-02-03 r/5828 fix(tvix/eval): ensure all evaluated thunks are correctly memoizedVincent Ambo1-2/+1
This fixes a very complicated bug (b/246). Evaluation progresses *much* further after this, leading to several less complicated bugs likely being uncovered by this What was the problem? ===================== Previously, when evaluating a thunk, we had a code path that looked like this: match *thunk { ThunkRepr::Evaluated(Value::Thunk(ref inner_thunk)) => { let inner_repr = inner_thunk.0.borrow().clone(); drop(thunk); self.0.replace(inner_repr); } /* ... */ } This code path created a copy of the inner `ThunkRepr` of a nested thunk, and moved that copy into the `ThunkRepr` of the parent. The effect of this was that the original `ThunkRepr` (unforced!) lived on in the original thunk, without the memoization of the subsequent forcing applying to it. This had the result that Tvix would repeatedly evaluate these thunks without ever memoizing them, if they occured repeatedly as shared inner thunks. Most notably, this would *always* occur when builtins.import was used. What's the solution? ==================== I have completely rewritten `Thunk::force_trampoline_self` to make all flows that can occur in it explicit. I have also removed the outer loop inside of that function, and resorted to more use of trampolining instead. The function is now well-commented and it should be possible to read it from top-to-bottom and get a general sense of what is going on, though the trampolining itself (which is implemented in the VM) needs to be at least partially understood for this. What's the new problem(s)? ========================== One new (known) problem is that we have to construct `Error` instances in all error types here, but we do not have spans available in some thunk-related situations. Due to b/238 we cannot ask the VM for an arbitrary span from the callsite leading to the force. This means that there are now code paths where, under certain conditions, causing an evaluation error during thunk forcing will panic. To fix this we will need to investigate and fix b/238, and/or add a span tracking mechanism to thunks themselves. What other impacts does this have? ================================== With this commit, eval of nixpkgs mostly succeeds (things like stdenv evaluate to the same hashes for us and C++ Nix, meaning we now construct identical derivations without eval breaking). Due to this we progress much further into nixpkgs, which lets us uncover more additional bugs. For example, after this commit we can quickly see that cl/7949 introduces some kind of behavioural issue and should not be merged as-is (this was not apparent before). Additionally, tvix-eval is now seemingly very fast. When doing performance analysis of a nixpkgs eval, we now mostly see the code path for shelling out to C++ Nix to add things to the store in there. We still need those code paths, so we can not (yet) do a performance analysis beyond that. Change-Id: I738525bad8bc5ede5d8c737f023b14b8f4160612 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8012 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-02-02 r/5823 fix(tvix/eval): unsafeDiscardStringContext is a no-opVincent Ambo1-4/+2
... not just a TODO. Most use-cases of unsafeDiscardStringContext are for cases where a string is processed in some ways and no longer contains a "physical" reference, but still has its context attached in C++ Nix. We don't need to do this. This does diverge in behaviour in use-cases related to build scheduling, but that whole behaviour will be different in Tvix. Change-Id: I4056d4c09f62d44d6bd52b791db03fe5556672b5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8016 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-31 r/5795 fix(tvix/eval): allow builtins.toXML to serialise any functionVincent Ambo1-1/+13
This adds a fake argument name to builtins.toXML which allows toXML to serialise any value instead of panicking on functions. We do still have to fix the value itself, eventually, though. Change-Id: I2e330ecddcd80442b4fac5eced64431ac86123ba Reviewed-on: https://cl.tvl.fyi/c/depot/+/7962 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-25 r/5754 feat(tvix/eval): implement builtins.fromTOMLFlorian Klink1-0/+7
This allows parsing TOML from Tvix. We can enable the eval-okay-fromTOML testcase from nix_tests. It uses the `toml` crate, and the serde integration it brings with it. Change-Id: Ic6f95aacf2aeb890116629b409752deac49dd655 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7920 Autosubmit: flokli <flokli@flokli.de> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-20 r/5707 refactor(tvix/eval): directly return builtin tuples from macroVincent Ambo2-16/+3
All invocations of the builtin macro had to previously filter through the `builtin_tuple` function, but it's more sensible to directly return these from the macro. Change-Id: I45600ba84d56c9528d3e92570461c319eea595ce Reviewed-on: https://cl.tvl.fyi/c/depot/+/7825 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2023-01-17 r/5675 refactor(tvix/eval): remove `Box` in new_suspended_nativeVincent Ambo1-2/+2
This is unnecessary, Rc already provides all the boxing we need. Change-Id: I08cf0939c48da43f04c847526c7e5dae5336d528 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7749 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-16 r/5664 feat(tvix/eval): implement builtins.toXMLVincent Ambo2-0/+135
Change-Id: I009efc53a8e98f0650ae660c4decd8216e8a06e7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7835 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-12 r/5654 fix(tvix/eval): len_without_is_empty clippy warnAaqa Ishtyaq1-1/+1
This CL addresses clippy warning len_without_is_empty which expects `.is_empty()` method to be present when implementing `.len()` method for an item. Change-Id: I8878db630b9ef5853649a906b764a33299bb5dc8 Signed-off-by: Aaqa Ishtyaq <aaqaishtyaq@gmail.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7806 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI
2023-01-12 r/5652 feat(tvix/eval): implement builtins.toJSONVincent Ambo1-0/+11
Implements `Serialize` for `tvix_eval::Value`. Special care is taken with serialisation of attribute sets, and forcing of thunks. The tests should cover both cases well. Change-Id: I9bb135bacf6f87bc6bd0bd88cef0a42308e6c335 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7803 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-10 r/5640 feat(tvix/eval): implement serde::Deserialize for ValueRyan Lahfa1-2/+2
Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: Ib6f7d1f4f4faac36b44f5f75cccc57bf912cf606 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7626 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-08 r/5629 fix(tvix/eval): fix last uses of Vec<Value> -> NixList in builtinsVincent Ambo1-11/+15
Change-Id: I0d71b82eb7ddc1e457b0996b0668006f55f56751 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7790 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2023-01-06 r/5620 refactor(tvix/eval): use builtins macro for placeholdersVincent Ambo1-67/+45
Change-Id: I30bc475e3e36a163fa169083481cdd4b4d0ca456 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7785 Reviewed-by: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Autosubmit: tazjin <tazjin@tvl.su>
2023-01-06 r/5619 refactor(tvix/eval): move mocked builtins.derivation to testsVincent Ambo1-34/+1
This placeholder should not live in the main crate anymore as we will be injecting the real one from outside of eval, but there are still language tests that depend on a (simple, mockable) version of it. Change-Id: I68ea169db15cbdbeed320930d3069e21e376c90d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7783 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2023-01-06 r/5595 feat(tvix/eval): add builtins.{null,true,false}sterni1-0/+3
Code probably rarely relies on these, but it's not hard to support them. Change-Id: I8499fec34efaf031f9c013bbd370a13db929a2a3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7772 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: sterni <sternenseemann@systemli.org>
2023-01-04 r/5581 refactor(tvix/eval): streamline construction of globals/builtinsVincent Ambo2-191/+50
Previously the construction of globals (a compiler-only concept) and builtins (a (now) user-facing API) was intermingled between multiple different modules, and kind of difficult to understand. The complexity of this had grown in large part due to the implementation of `builtins.import`, which required the notorious "knot-tying" trick using Rc::new_cyclic (see cl/7097) for constructing the set of globals. As part of the new `Evaluation` API users should have the ability to bring their own builtins, and control explicitly whether or not impure builtins are available (regardless of whether they're compiled in or not). To streamline the construction and allow the new API features to work, this commit restructures things by making these changes: 1. The `tvix_eval::builtins` module is now only responsible for exporting sets of builtins. It no longer has any knowledge of whether or not certain sets (e.g. only pure, or pure+impure) are enabled, and it has no control over which builtins are globally available (this is now handled in the compiler). 2. The compiler module is now responsible for both constructing the final attribute set of builtins from the set of builtins supplied by a user, as well as for populating its globals (that is identifiers which are available at the top-level scope). 3. The `Evaluation` API now carries a `builtins` field which is populated with the pure builtins by default, and can be extended by users. 4. The `import` feature has been moved into the compiler, as a special case. In general, builtins no longer have the ability to reference the "fix point" of the globals set. This should not change any functionality, and in fact preserves minor differences between Tvix/Nix that we already had (such as `builtins.builtins` not existing). Change-Id: Icdf5dd50eb81eb9260d89269d6e08b1e67811a2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7738 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de>
2022-12-29 r/5542 refactor(tvix/eval): remove extra Rc<..> around Value::AttrsVincent Ambo1-4/+4
The `im::OrdMap` is already small and cheap to copy while sharing memory, so this is not required anymore. Only the `KV` variant may have slightly larger content, but in practice this doesn't seem to make a difference when comparing the two variants and this one is less complicated. Change-Id: I64a563b209a2444125653777551373cb2989ca7d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7677 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5541 refactor(tvix/eval): persistent, memory-sharing OrdMap for NixAttrsVincent Ambo1-12/+12
This uses the `im::OrdMap` for `NixAttrs` to enable sharing of memory between different iterations of a map. This slightly speeds up eval, but not significantly. Future work might include benchmarking whether using a `HashMap` and only ordering in cases where order is actually required would help. This switches to a fork of `im` that fixes some bugs with its OrdMap implementation. Change-Id: I2f6a5ff471b6d508c1e8a98b13f889f49c0d9537 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7676 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5540 refactor(tvix/eval): use im::Vector directly where possibleVincent Ambo1-15/+18
The conversion from im::Vector -> Vec is cheaper for NixList construction (of course), so where possible we should make use of that. This updates most builtins dealing with lists to use Vector directly, and marks the function constructing NixList from Vec as deprecated so that we get appropriate warnings in places where it's still in use. These places are currently inside of JSON serialisation logic which is in flux right now, so lets leave them as-is until it's stabilised. Change-Id: I037f12a2800f2576db4d9526bd935efd079163f0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7671 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-12-29 r/5534 refactor(tvix/eval): use im::Vector for NixList representationVincent Ambo1-1/+6
This is a persistent, structurally sharing data structure which is more efficient in some of our use-cases. I have verified the efficiency improvement using `hyperfine` repeatedly over expressions on nixpkgs. Lists are not the most performance-critical structure in Nix (that would be attribute sets), but we can already see a small (~5-10%) improvement. Note that there are a handful of cases where we still go via `Vec` that need to be fixed, most notable for `builtins.sort` which can not currently be implemented directly using `im::Vector` because of a restrictive type bound. Change-Id: I237cc50cbd7629a046e5a5e4601fbb40355e551d Reviewed-on: https://cl.tvl.fyi/c/depot/+/7670 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-12-25 r/5486 fix(tvix/eval): fix current clippy warningsVincent Ambo1-11/+6
It's been a while since the last time, so quite a lot of stuff has accumulated here. Change-Id: I0762827c197b30a917ff470fd8ae8f220f6ba247 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7597 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-12-21 r/5466 refactor(tvix/eval): use light spans in builtins.importVincent Ambo1-6/+2
Change-Id: I05732073155b430575babb6f076bf465aef98857 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7581 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-12-21 r/5465 feat(tvix/eval): builtins.storeDirAdam Joseph1-1/+11
Returns the store directory through EvalIO::store_dir. Note that this is _optional_ in Tvix, as an evaluation can occur in a context where there simply is no store directory. In those contexts, `builtins.storeDir` returns `null` in Tvix. This would only happen in contexts like Tvixbolt (or completely unrelated use-cases) in practice. Co-Authored-By: Vincent Ambo <tazjin@tvl.su> Change-Id: I5a752c7e89b2f75bd7efb082dbfa5b25e3b1ff3b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7452 Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-12-21 r/5464 refactor(tvix/eval): use `EvalIO::read_dir` for equivalent builtinVincent Ambo1-28/+20
Change-Id: I6d782c07166f51587d2f1d06607823268debb5d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7574 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5463 refactor(tvix/eval): use `EvalIO::path_exists` for the builtinVincent Ambo1-1/+2
Change-Id: I49822ce30137777865e7370ee86666636e277b35 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7573 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-12-21 r/5460 refactor(tvix/eval): use EvalIO::read_to_string in impure builtinsVincent Ambo1-21/+16
With this change, the behaviour of reading a string from a file path is controlled by the provided `EvalIO` structure. This is a huge step towards abstracting away I/O behaviour correctly. Change-Id: Ifde8e46cd863b16e0301dca45a434ad27560399f Reviewed-on: https://cl.tvl.fyi/c/depot/+/7567 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-12-21 r/5457 refactor(tvix/eval): add a LightSpan type for lighter span trackingVincent Ambo1-1/+2
This type carries the information required for calculating a span (i.e. the chunk and offset), instead of the span itself. The span is then only calculated in cases where it is required (when throwing errors). This reduces the eval time for `builtins.length (builtins.attrNames (import <nixpkgs> {}))` by *one third*! The data structure in chunks that carries span information reduces in-memory size by trading off the speed of retrieving span information. This is because the span information is only actually required when throwing errors (or emitting warnings). However, somewhere along the way we grew a dependency on carrying span information in thunks (for correctly reporting error chains). Hitting the code paths for span retrieval was expensive, and carrying the spans in a different way would still be less cache-efficient. This change is the best tradeoff I could come up with. Refs: b/229. Change-Id: I27d4c4b5c5f9be90ac47f2db61941e123a78a77b Reviewed-on: https://cl.tvl.fyi/c/depot/+/7558 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-21 r/5452 feat(tvix/eval): wrap Closure in Rc<> to match cppnix semanticsAdam Joseph1-1/+1
Change-Id: I595087eff943d38a9fc78a83d37e207bb2ab79bc Reviewed-on: https://cl.tvl.fyi/c/depot/+/7443 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-12-03 r/5379 feat(tvix/eval): Continue removing leakage of BTreeMap.Lyle Mantooth2-42/+37
Fixes b/212. Based on feedback in https://cl.tvl.fyi/c/depot/+/7492, all uses of `NixAttrs::from_map` have been removed. Only `from_iter` and `from_kv` remain. Change-Id: I52e25f73018c2aa1843197427516b7a852503e2c Reviewed-on: https://cl.tvl.fyi/c/depot/+/7500 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Autosubmit: IslandUsurper <lyle@menteeth.us>
2022-12-02 r/5375 feat(tvix/eval): impl FromIterator for NixAttrsLyle Mantooth1-52/+46
Allows for the removal of some BTreeMap usage when constructing NixAttrs by allowing any iterator over 2-tuples to build a NixAttrs. Some instances of BTreeMap didn't have anything to do with making NixAttrs, and some were just the best tool for the job, so they are left using the old `from_map` interface. Change-Id: I668ea600b0d93eae700a6b1861ac84502c968d78 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7492 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-12-02 r/5366 feat(tvix/eval): crude caching builtins.importAdam Joseph1-6/+16
Before this, tvix was spending most of its time furiously re-parsing and re-compiling nixpkgs, each time hoping to get a different result... Change-Id: I1c0cfbf9af622c276275b1f2fb8d4e976f1b5533 Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7361 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-12-01 r/5356 feat(tvix/eval): placeholder for builtins.placeholderAdam Joseph1-0/+7
Change-Id: I8d11f2db4489a7d82910256069d10f8bed3bdf9a Reviewed-on: https://cl.tvl.fyi/c/depot/+/7451 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2022-11-27 r/5345 feat(tvix/eval): non-recursive implementation of nix_eq()Adam Joseph1-1/+1
This passes all the function/thunk-pointer-equality tests in cl/7369. Change-Id: Ib47535ba2fc77a4f1c2cc2fd23d3a879e21d8b4c Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7358 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-11-26 r/5327 feat(tvix/eval): mock builtins.unsafeGetAttrPosAdam Joseph1-0/+24
Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I9d986dd8c0aad4e67df01bda13cee443e0fc0d20 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7415 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-11-23 r/5301 feat(tvix/eval): make NixList::clone() cheapAdam Joseph1-12/+13
When we start unrecursivifying (sp?) things, Rust's borrow checker is going to be a headache; its magic only works when you use the CPU stack as your call stack. Fixing the borrow checker issues usually involves adding lots of `clone()`s. Right now `NixList` is the only variant of `Value` that isn't cheap to clone() -- all the others are either a wrapper around Rc or else are of bounded size. Note that this requires dropping the `DerefMut for NixList` instance and using `Vec<Value>` instead in those situations. Change-Id: I5a47df66855342aa2064f8f3cb7934ff422d26bd Signed-off-by: Adam Joseph <adam@westernsemico.com> Reviewed-on: https://cl.tvl.fyi/c/depot/+/7359 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
generated by cgit-pink 1.4.1 (git 2.44.2) at 2024-11-22T15·29+0000