Remove RegisterBufCache since it didn't really measurably help at all

This commit is contained in:
Devyn Cairns 2024-07-05 20:44:05 -07:00
parent 4343d1d80c
commit be8637d6cc
No known key found for this signature in database
4 changed files with 10 additions and 87 deletions

View File

@ -24,7 +24,12 @@ pub fn eval_ir_block<D: DebugContext>(
let args_base = stack.arguments.get_base();
let error_handler_base = stack.error_handlers.get_base();
let mut registers = stack.register_buf_cache.acquire(ir_block.register_count);
// Allocate and initialize registers. I've found that it's not really worth trying to avoid
// the heap allocation here by reusing buffers - our allocator is fast enough
let mut registers = Vec::with_capacity(ir_block.register_count);
let empty = std::iter::repeat_with(|| PipelineData::Empty);
registers.extend(empty.take(ir_block.register_count));
let result = eval_ir_block_impl::<D>(
&mut EvalContext {
@ -44,7 +49,6 @@ pub fn eval_ir_block<D: DebugContext>(
input,
);
stack.register_buf_cache.release(registers);
stack.error_handlers.leave_frame(error_handler_base);
stack.arguments.leave_frame(args_base);
@ -128,12 +132,6 @@ impl<'a> EvalContext<'a> {
let mut new_stack = Box::new(self.stack.gather_captures(self.engine_state, &[]));
// Swap the RegisterBufCache onto the new stack so that we can reuse the buffers
std::mem::swap(
&mut self.stack.register_buf_cache,
&mut new_stack.register_buf_cache,
);
// Increment recursion count on callee stack to prevent recursing too far
new_stack.recursion_count += 1;
@ -142,13 +140,8 @@ impl<'a> EvalContext<'a> {
/// Drop the callee stack, so that arguments will be put on the caller stack instead
fn drop_callee_stack(&mut self) {
if let Some(mut callee_stack) = self.callee_stack.take() {
// Swap the RegisterBufCache onto our stack so that we can reuse the buffers
std::mem::swap(
&mut callee_stack.register_buf_cache,
&mut self.stack.register_buf_cache,
);
}
// this is provided so in case some other cleanup needs to be done, it can be done here.
self.callee_stack = None;
}
}

View File

@ -8,7 +8,6 @@ mod engine_state;
mod error_handler;
mod overlay;
mod pattern_match;
mod register_buf_cache;
mod stack;
mod stack_out_dest;
mod state_delta;
@ -27,7 +26,6 @@ pub use engine_state::*;
pub use error_handler::*;
pub use overlay::*;
pub use pattern_match::*;
pub use register_buf_cache::*;
pub use stack::*;
pub use stack_out_dest::*;
pub use state_delta::*;

View File

@ -1,62 +0,0 @@
use std::fmt;
use crate::PipelineData;
/// Retains buffers for reuse in IR evaluation, avoiding heap allocation.
///
/// This is implemented in such a way that [`Clone`] is still possible, by making the fact that the
/// buffers can't be preserved on clone completely transparent. The cached buffers are always empty.
pub struct RegisterBufCache {
bufs: Vec<Vec<PipelineData>>,
}
// SAFETY: because `bufs` only ever contains empty `Vec`s, it doesn't actually contain any of the
// data.
unsafe impl Send for RegisterBufCache {}
unsafe impl Sync for RegisterBufCache {}
impl RegisterBufCache {
/// Create a new cache with no register buffers.
pub const fn new() -> Self {
RegisterBufCache { bufs: vec![] }
}
/// Acquire a new register buffer from the cache. The buffer will be extended to `size` with
/// [`Empty`](PipelineData::Empty) elements.
pub fn acquire(&mut self, size: usize) -> Vec<PipelineData> {
let mut buf = if let Some(buf) = self.bufs.pop() {
debug_assert!(buf.is_empty());
buf
} else {
Vec::new()
};
buf.reserve(size);
buf.extend(std::iter::repeat_with(|| PipelineData::Empty).take(size));
buf
}
/// Release a used register buffer to the cache. The buffer will be cleared.
pub fn release(&mut self, mut buf: Vec<PipelineData>) {
// SAFETY: this `clear` is necessary for the `unsafe impl`s to be safe
buf.clear();
self.bufs.push(buf);
}
}
impl fmt::Debug for RegisterBufCache {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let bufs = self.bufs.len();
let bytes: usize = self
.bufs
.iter()
.map(|b| b.capacity() * std::mem::size_of::<PipelineData>())
.sum();
write!(f, "RegisterBufCache({bufs} bufs, {bytes} bytes)")
}
}
impl Clone for RegisterBufCache {
fn clone(&self) -> Self {
RegisterBufCache::new()
}
}

View File

@ -1,7 +1,7 @@
use crate::{
engine::{
ArgumentStack, EngineState, ErrorHandlerStack, Redirection, RegisterBufCache,
StackCallArgGuard, StackCaptureGuard, StackIoGuard, StackOutDest, DEFAULT_OVERLAY_NAME,
ArgumentStack, EngineState, ErrorHandlerStack, Redirection, StackCallArgGuard,
StackCaptureGuard, StackIoGuard, StackOutDest, DEFAULT_OVERLAY_NAME,
},
OutDest, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, NU_VARIABLE_ID,
};
@ -41,8 +41,6 @@ pub struct Stack {
pub env_hidden: HashMap<String, HashSet<String>>,
/// List of active overlays
pub active_overlays: Vec<String>,
/// Cached register buffers for IR evaluation
pub register_buf_cache: RegisterBufCache,
/// Argument stack for IR evaluation
pub arguments: ArgumentStack,
/// Error handler stack for IR evaluation
@ -76,7 +74,6 @@ impl Stack {
env_vars: Vec::new(),
env_hidden: HashMap::new(),
active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()],
register_buf_cache: RegisterBufCache::new(),
arguments: ArgumentStack::new(),
error_handlers: ErrorHandlerStack::new(),
use_ir: false,
@ -97,7 +94,6 @@ impl Stack {
env_vars: parent.env_vars.clone(),
env_hidden: parent.env_hidden.clone(),
active_overlays: parent.active_overlays.clone(),
register_buf_cache: RegisterBufCache::new(),
arguments: ArgumentStack::new(),
error_handlers: ErrorHandlerStack::new(),
use_ir: parent.use_ir,
@ -270,7 +266,6 @@ impl Stack {
env_vars,
env_hidden: self.env_hidden.clone(),
active_overlays: self.active_overlays.clone(),
register_buf_cache: RegisterBufCache::new(),
arguments: ArgumentStack::new(),
error_handlers: ErrorHandlerStack::new(),
use_ir: self.use_ir,
@ -304,7 +299,6 @@ impl Stack {
env_vars,
env_hidden: self.env_hidden.clone(),
active_overlays: self.active_overlays.clone(),
register_buf_cache: RegisterBufCache::new(),
arguments: ArgumentStack::new(),
error_handlers: ErrorHandlerStack::new(),
use_ir: self.use_ir,