refactor(tvix/eval): add a LightSpan type for lighter span tracking
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
This commit is contained in:
parent
8b3d03db92
commit
bf286a54bc
5 changed files with 61 additions and 12 deletions
|
@ -12,6 +12,7 @@ use crate::{
|
||||||
compiler::GlobalsMap,
|
compiler::GlobalsMap,
|
||||||
errors::ErrorKind,
|
errors::ErrorKind,
|
||||||
observer::NoOpObserver,
|
observer::NoOpObserver,
|
||||||
|
spans::LightSpan,
|
||||||
value::{Builtin, BuiltinArgument, NixAttrs, Thunk},
|
value::{Builtin, BuiltinArgument, NixAttrs, Thunk},
|
||||||
vm::VM,
|
vm::VM,
|
||||||
SourceCode, Value,
|
SourceCode, Value,
|
||||||
|
@ -176,7 +177,7 @@ pub fn builtins_import(globals: &Weak<GlobalsMap>, source: SourceCode) -> Builti
|
||||||
let res = entry
|
let res = entry
|
||||||
.insert(Value::Thunk(Thunk::new_suspended(
|
.insert(Value::Thunk(Thunk::new_suspended(
|
||||||
result.lambda,
|
result.lambda,
|
||||||
current_span,
|
LightSpan::new_actual(current_span),
|
||||||
)))
|
)))
|
||||||
.clone();
|
.clone();
|
||||||
|
|
||||||
|
|
|
@ -28,6 +28,7 @@ use crate::chunk::Chunk;
|
||||||
use crate::errors::{Error, ErrorKind, EvalResult};
|
use crate::errors::{Error, ErrorKind, EvalResult};
|
||||||
use crate::observer::CompilerObserver;
|
use crate::observer::CompilerObserver;
|
||||||
use crate::opcode::{CodeIdx, Count, JumpOffset, OpCode, UpvalueIdx};
|
use crate::opcode::{CodeIdx, Count, JumpOffset, OpCode, UpvalueIdx};
|
||||||
|
use crate::spans::LightSpan;
|
||||||
use crate::spans::ToSpan;
|
use crate::spans::ToSpan;
|
||||||
use crate::value::{Closure, Formals, Lambda, Thunk, Value};
|
use crate::value::{Closure, Formals, Lambda, Thunk, Value};
|
||||||
use crate::warnings::{EvalWarning, WarningKind};
|
use crate::warnings::{EvalWarning, WarningKind};
|
||||||
|
@ -946,7 +947,7 @@ impl Compiler<'_> {
|
||||||
if lambda.upvalue_count == 0 {
|
if lambda.upvalue_count == 0 {
|
||||||
self.emit_constant(
|
self.emit_constant(
|
||||||
if is_suspended_thunk {
|
if is_suspended_thunk {
|
||||||
Value::Thunk(Thunk::new_suspended(lambda, span))
|
Value::Thunk(Thunk::new_suspended(lambda, LightSpan::new_actual(span)))
|
||||||
} else {
|
} else {
|
||||||
Value::Closure(Rc::new(Closure::new(lambda)))
|
Value::Closure(Rc::new(Closure::new(lambda)))
|
||||||
},
|
},
|
||||||
|
|
|
@ -1,9 +1,46 @@
|
||||||
//! Utilities for dealing with span tracking in the compiler and in
|
//! Utilities for dealing with span tracking in the compiler and in
|
||||||
//! error reporting.
|
//! error reporting.
|
||||||
|
|
||||||
|
use crate::opcode::CodeIdx;
|
||||||
|
use crate::value::Lambda;
|
||||||
use codemap::{File, Span};
|
use codemap::{File, Span};
|
||||||
use rnix::ast;
|
use rnix::ast;
|
||||||
use rowan::ast::AstNode;
|
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.
|
||||||
|
///
|
||||||
|
/// This is used for tracking spans across thunk boundaries, as they
|
||||||
|
/// are frequently instantiated but spans are only used in error or
|
||||||
|
/// warning cases.
|
||||||
|
#[derive(Clone, Debug)]
|
||||||
|
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 }
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn span(&self) -> Span {
|
||||||
|
match self {
|
||||||
|
LightSpan::Actual { span } => *span,
|
||||||
|
LightSpan::Delayed { lambda, offset } => lambda.chunk.get_span(*offset),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Trait implemented by all types from which we can retrieve a span.
|
/// Trait implemented by all types from which we can retrieve a span.
|
||||||
pub trait ToSpan {
|
pub trait ToSpan {
|
||||||
|
|
|
@ -24,11 +24,10 @@ use std::{
|
||||||
rc::Rc,
|
rc::Rc,
|
||||||
};
|
};
|
||||||
|
|
||||||
use codemap::Span;
|
|
||||||
|
|
||||||
use crate::{
|
use crate::{
|
||||||
chunk::Chunk,
|
chunk::Chunk,
|
||||||
errors::{Error, ErrorKind},
|
errors::{Error, ErrorKind},
|
||||||
|
spans::LightSpan,
|
||||||
upvalues::Upvalues,
|
upvalues::Upvalues,
|
||||||
value::{Builtin, Closure},
|
value::{Builtin, Closure},
|
||||||
vm::VM,
|
vm::VM,
|
||||||
|
@ -49,7 +48,7 @@ enum ThunkRepr {
|
||||||
Suspended {
|
Suspended {
|
||||||
lambda: Rc<Lambda>,
|
lambda: Rc<Lambda>,
|
||||||
upvalues: Rc<Upvalues>,
|
upvalues: Rc<Upvalues>,
|
||||||
span: Span,
|
light_span: LightSpan,
|
||||||
},
|
},
|
||||||
|
|
||||||
/// Thunk currently under-evaluation; encountering a blackhole
|
/// Thunk currently under-evaluation; encountering a blackhole
|
||||||
|
@ -77,11 +76,11 @@ impl Thunk {
|
||||||
)))))
|
)))))
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn new_suspended(lambda: Rc<Lambda>, span: Span) -> Self {
|
pub fn new_suspended(lambda: Rc<Lambda>, light_span: LightSpan) -> Self {
|
||||||
Thunk(Rc::new(RefCell::new(ThunkRepr::Suspended {
|
Thunk(Rc::new(RefCell::new(ThunkRepr::Suspended {
|
||||||
upvalues: Rc::new(Upvalues::with_capacity(lambda.upvalue_count)),
|
upvalues: Rc::new(Upvalues::with_capacity(lambda.upvalue_count)),
|
||||||
lambda: lambda.clone(),
|
lambda: lambda.clone(),
|
||||||
span,
|
light_span,
|
||||||
})))
|
})))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -124,7 +123,7 @@ impl Thunk {
|
||||||
Thunk(Rc::new(RefCell::new(ThunkRepr::Suspended {
|
Thunk(Rc::new(RefCell::new(ThunkRepr::Suspended {
|
||||||
lambda: Rc::new(lambda),
|
lambda: Rc::new(lambda),
|
||||||
upvalues: Rc::new(Upvalues::with_capacity(0)),
|
upvalues: Rc::new(Upvalues::with_capacity(0)),
|
||||||
span: span,
|
light_span: LightSpan::new_actual(span),
|
||||||
})))
|
})))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -151,12 +150,16 @@ impl Thunk {
|
||||||
if let ThunkRepr::Suspended {
|
if let ThunkRepr::Suspended {
|
||||||
lambda,
|
lambda,
|
||||||
upvalues,
|
upvalues,
|
||||||
span,
|
light_span,
|
||||||
} = std::mem::replace(&mut *thunk_mut, ThunkRepr::Blackhole)
|
} = std::mem::replace(&mut *thunk_mut, ThunkRepr::Blackhole)
|
||||||
{
|
{
|
||||||
drop(thunk_mut);
|
drop(thunk_mut);
|
||||||
vm.enter_frame(lambda, upvalues, 0)
|
vm.enter_frame(lambda, upvalues, 0).map_err(|e| {
|
||||||
.map_err(|e| ErrorKind::ThunkForce(Box::new(Error { span, ..e })))?;
|
ErrorKind::ThunkForce(Box::new(Error {
|
||||||
|
span: light_span.span(),
|
||||||
|
..e
|
||||||
|
}))
|
||||||
|
})?;
|
||||||
(*self.0.borrow_mut()) = ThunkRepr::Evaluated(vm.pop())
|
(*self.0.borrow_mut()) = ThunkRepr::Evaluated(vm.pop())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -10,6 +10,7 @@ use crate::{
|
||||||
nix_search_path::NixSearchPath,
|
nix_search_path::NixSearchPath,
|
||||||
observer::RuntimeObserver,
|
observer::RuntimeObserver,
|
||||||
opcode::{CodeIdx, Count, JumpOffset, OpCode, StackIdx, UpvalueIdx},
|
opcode::{CodeIdx, Count, JumpOffset, OpCode, StackIdx, UpvalueIdx},
|
||||||
|
spans::LightSpan,
|
||||||
unwrap_or_clone_rc,
|
unwrap_or_clone_rc,
|
||||||
upvalues::Upvalues,
|
upvalues::Upvalues,
|
||||||
value::{Builtin, Closure, CoercionKind, Lambda, NixAttrs, NixList, Thunk, Value},
|
value::{Builtin, Closure, CoercionKind, Lambda, NixAttrs, NixList, Thunk, Value},
|
||||||
|
@ -211,6 +212,12 @@ impl<'o> VM<'o> {
|
||||||
self.chunk().get_span(self.frame().ip - 1)
|
self.chunk().get_span(self.frame().ip - 1)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the information needed to calculate the current span,
|
||||||
|
/// but without performing that calculation.
|
||||||
|
fn current_light_span(&self) -> LightSpan {
|
||||||
|
LightSpan::new_delayed(self.frame().lambda.clone(), self.frame().ip - 1)
|
||||||
|
}
|
||||||
|
|
||||||
/// Construct an error from the given ErrorKind and the source
|
/// Construct an error from the given ErrorKind and the source
|
||||||
/// span of the current instruction.
|
/// span of the current instruction.
|
||||||
pub fn error(&self, kind: ErrorKind) -> Error {
|
pub fn error(&self, kind: ErrorKind) -> Error {
|
||||||
|
@ -884,7 +891,7 @@ impl<'o> VM<'o> {
|
||||||
);
|
);
|
||||||
Thunk::new_closure(blueprint)
|
Thunk::new_closure(blueprint)
|
||||||
} else {
|
} else {
|
||||||
Thunk::new_suspended(blueprint, self.current_span())
|
Thunk::new_suspended(blueprint, self.current_light_span())
|
||||||
};
|
};
|
||||||
let upvalues = thunk.upvalues_mut();
|
let upvalues = thunk.upvalues_mut();
|
||||||
self.push(Value::Thunk(thunk.clone()));
|
self.push(Value::Thunk(thunk.clone()));
|
||||||
|
|
Loading…
Reference in a new issue