about summary refs log tree commit diff
path: root/OWNERS
diff options
context:
space:
mode:
authorAdam Joseph <adam@westernsemico.com>2023-12-09T06·33-0800
committerclbot <clbot@tvl.fyi>2023-12-12T14·34+0000
commitc92d06271d30394ec09ae3046efe4b671a7e266f (patch)
treef30e2ac974b8bfbff988f0a07f58604a95b4d639 /OWNERS
parent29878259b33db2508566dea862b9767619ff0d5d (diff)
feat(tvix/eval): drop LightSpan::Delayed r/7172
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
Diffstat (limited to 'OWNERS')
0 files changed, 0 insertions, 0 deletions