Commit graph

15 commits

Author SHA1 Message Date
sterni
f3c27f1717 fix(tvix/eval): bring foldl' strictness in line with C++ Nix
Working on https://github.com/NixOS/nix/pull/7158, I discovered that C++
Nix actually is strict in the accumulator, just not in the first value.
This seems due to the fact that in the C++ evaluator, function calls
don't seem to be thunked unconditionally and foldl' just elects not to
wrap it in a thunk (don't quote me on this summary, even though it seems
to line up with the code for primop_foldlStrict and testable behavior).

It doesn't seem worth it to risk breaking the odd Nix expression just to
be strict in one more value per invocation of foldl' (i.e. the initial
accumulator value `nul`), so let's match the existing C++ Nix behavior
here.

Change-Id: If59e62271a90d97cb440f0ca72a58ec7840d1690
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7022
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-15 14:12:23 +00:00
sterni
fcd5e53703 fix(tvix/eval/builtins): force acc not list element in foldl'
When investigating discrepancies between foldl' in tvix and C++ Nix,
I discovered that C++ Nix's foldl' doesn't seem to be strict at all.
Since this seemed wrong, I looked into Haskell's foldl' implementation
which doesn't force the list elements (`val` in our code), but the
accumulation value (`res` in our code). You can look at the code here:
https://hackage.haskell.org/package/base-4.17.0.0/docs/src/GHC.List.html#foldl%27

This actually makes a lot of sense: If `res` is not forced after each
application of `op`, we'll end up thunks nested as deeply as the list is
long, potentially taking up a lot of space. This can be limited by
forcing the `res` thunk before applying `op` again (and creating a new
thunk).

I've also PR-ed an equivalent change for C++ Nix at
https://github.com/NixOS/nix/pull/7158. Since this is not merged nor
backported to our Nix 2.3 fork, I've not copied the eval fail test yet,
since it wouldn't when checking our tests against C++ Nix in depot.

Change-Id: I34edf6fc3031fc1485c3e714f2280b4fba8f004b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6947
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
2022-10-11 15:53:29 +00:00
sterni
1e25ba1b09 docs(tvix/eval): start doc about problematic/weird lang behavior
The idea is that we can keep track of the more unexpected behavior,
behavior that maybe should not be a thing at all and behavior we are not
sure about yet.

Change-Id: I70933f00af1230a7ab9d30e917b61199fe571caf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6803
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-01 12:30:17 +00:00
sterni
a798efc77b docs(tvix/eval): note C++ implementation details for C++ Nix
Just want to note those down somewhere before I forget them again,
we can delete them later if we have it all figured out.

Change-Id: Icafa2d8fc7ca39e38e9637b7eca6f2bbf487c2b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6632
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-18 15:01:34 +00:00
sterni
b6ab02e48b docs(tvix/eval): propose builtin "inlining" optimisation
Change-Id: I96a187792a1fd48cffd6b56ec22347aee8cae3af
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6526
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-09-11 20:02:46 +00:00
sterni
cbde0292b6 docs(tvix/eval): mention ? and or for builtins optimisation
Change-Id: Ifaa6da345d408a69ce46d6a0e7483352715c75bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6525
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
2022-09-11 20:02:46 +00:00
Vincent Ambo
b4309f5b8a docs(tvix/eval): add some notes on recursive attribute sets
Change-Id: I36b826f12854a22e60a27ed1982ab5528c58bdad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6489
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 12:26:23 +00:00
Vincent Ambo
e944c341a7 docs(tvix/eval): add optimisation note on eliminating with thunks
Change-Id: I18d50ac8e157929a027f8bf284e65f1eb8950d5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6488
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-11 12:26:23 +00:00
Vincent Ambo
2246a31e72 refactor(tvix/eval): return call frame result from VM::call
Previously, "calling" (setting up the VM run loop for executing a call
frame) and "running" (running this loop to completion) were separate
operations.

This was basically an attempt to avoid nesting `VM::run` invocations.
However, doing things this way introduced some tricky bugs for exiting
out of the call frames of thunks vs. builtins & closures.

For now, we unify the two operations and always return the value to
the caller directly. For now this makes calls a little less effective,
but it gives us a chance to nail down some other strange behaviours
and then re-optimise this afterwards.

To make sure we tackle this again further down I've added it to the
list of known possible optimisations.

Change-Id: I96828ab6a628136e0bac1bf03555faa4e6b74ece
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6415
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2022-09-08 12:53:20 +00:00
Vincent Ambo
a3b19ad8be docs(tvix/eval): add notes for builtins access optimisation
Change-Id: Iadbfbe2864ae42fe5492ef3ede0925baee4872b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6413
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
2022-09-08 08:45:15 +00:00
Vincent Ambo
06b07f5c47 docs(tvix/eval): start a document on known optimisation potential
Change-Id: I9bc41e57e1cfdf536d7b9048bac2e7aff1ee2ffa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6313
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-06 07:29:25 +00:00
sterni
18efbac5c5 docs(tvix/eval): pad pure column so it can fit "false"
Change-Id: Iaedfc281db82de1e8eb2400db1118c8431d2579f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6333
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-04 21:05:57 +00:00
sterni
e25972877a docs(tvix/eval): add a list of builtins added in Nix >= 2.4
`builtins.getFlake` doesn't interest us, of course, but some others may
be worth (or easy) to implement. They are pretty low priority, though,
since nixpkgs has compatiblity wrappers for the ones it uses.

The new debugging-related builtins (break and traceVerbose) are
interesting to note, but may not make sense to implement at all.

Change-Id: Icae547aa3bd9d6ee6b87897ba8210eb9b9b044c7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6332
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-04 21:02:51 +00:00
Vincent Ambo
c48aa058a7 docs(tvix/eval): add an overview of all builtins in Nix
Change-Id: Ie187f3317046c6c9e59852d4a128f25ceed99309
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6252
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 12:59:23 +00:00
Vincent Ambo
6f13c16f28 docs(tvix/eval): add design documentation for attrset opcodes
Change-Id: I303b57e035543f4597c6247983d1d533e4014638
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6092
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
2022-08-13 15:31:50 +00:00