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

2 files changed, 1 insertions, 13 deletions

diff --git a/tvix/eval/src/spans.rs b/tvix/eval/src/spans.rs
index c0130a6654..f422093b0d 100644
--- a/tvix/eval/src/spans.rs
+++ b/tvix/eval/src/spans.rs
@@ -1,12 +1,9 @@
 //! Utilities for dealing with span tracking in the compiler and in
 //! error reporting.
 
-use crate::opcode::CodeIdx;
-use crate::value::Lambda;
 use codemap::{File, Span};
 use rnix::ast;
 use rowan::ast::AstNode;
-use std::rc::Rc;
 
 /// Helper struct to carry information required for making a span, but
 /// without actually performing the (expensive) span lookup.
@@ -19,17 +16,9 @@ pub enum LightSpan {
     /// The span has already been computed and can just be used right
     /// away.
     Actual { span: Span },
-
-    /// The span needs to be computed from the provided data, but only
-    /// when it is required.
-    Delayed { lambda: Rc<Lambda>, offset: CodeIdx },
 }
 
 impl LightSpan {
-    pub fn new_delayed(lambda: Rc<Lambda>, offset: CodeIdx) -> Self {
-        Self::Delayed { lambda, offset }
-    }
-
     pub fn new_actual(span: Span) -> Self {
         Self::Actual { span }
     }
@@ -37,7 +26,6 @@ impl LightSpan {
     pub fn span(&self) -> Span {
         match self {
             LightSpan::Actual { span } => *span,
-            LightSpan::Delayed { lambda, offset } => lambda.chunk.get_span(*offset),
         }
     }
 }
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index 99a913c46a..e11b3b5d13 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -161,7 +161,7 @@ impl CallFrame {
     /// but without performing that calculation.
     // TODO: why pub?
     pub(crate) fn current_light_span(&self) -> LightSpan {
-        LightSpan::new_delayed(self.lambda.clone(), self.ip - 1)
+        LightSpan::new_actual(self.current_span())
     }
 }