Skip to content

Pool temporary buffers in the resolution linearization path #998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dersam wants to merge 1 commit into
base: main
Choose a base branch
from
+123 −39
Open

Pool temporary buffers in the resolution linearization path #998

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 123 additions & 39 deletions rust/rubydex/src/resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ pub struct Resolver<'a> {
unit_queue: VecDeque<Unit>,
/// Whether we made any progress in the last pass of the resolution loop
made_progress: bool,
/// Spare buffers for the parent ancestor chains. Linearization recurses, and each level needs
/// one buffer, thus the pool keeps the buffers alive between calls to avoid one heap
/// allocation for each linearized declaration.
chain_pool: Vec<Vec<Ancestor>>,
/// Spare buffers for the collected mixins of a declaration, reused the same way as `chain_pool`.
mixin_pool: Vec<Vec<Mixin>>,
/// Spare deques for the linearized prepends and includes, reused the same way as `chain_pool`.
deque_pool: Vec<VecDeque<Ancestor>>,
/// The linearization context carried across recursion
context: LinearizationContext,
}
Expand All @@ -109,10 +117,46 @@ impl<'a> Resolver<'a> {
graph,
unit_queue: VecDeque::new(),
made_progress: false,
chain_pool: Vec::new(),
mixin_pool: Vec::new(),
deque_pool: Vec::new(),
context: LinearizationContext::new(),
}
}

/// Take an empty chain buffer from the pool, or make a new one if the pool is empty.
fn take_chain_buffer(&mut self) -> Vec<Ancestor> {
self.chain_pool.pop().unwrap_or_default()
}

/// Give a chain buffer back to the pool for the next linearization to use.
fn return_chain_buffer(&mut self, mut buffer: Vec<Ancestor>) {
buffer.clear();
self.chain_pool.push(buffer);
}

/// Take an empty mixin buffer from the pool, or make a new one if the pool is empty.
fn take_mixin_buffer(&mut self) -> Vec<Mixin> {
self.mixin_pool.pop().unwrap_or_default()
}

/// Give a mixin buffer back to the pool.
fn return_mixin_buffer(&mut self, mut buffer: Vec<Mixin>) {
buffer.clear();
self.mixin_pool.push(buffer);
}

/// Take an empty deque from the pool, or make a new one if the pool is empty.
fn take_deque(&mut self) -> VecDeque<Ancestor> {
self.deque_pool.pop().unwrap_or_default()
}

/// Give a deque back to the pool.
fn return_deque(&mut self, mut deque: VecDeque<Ancestor>) {
deque.clear();
self.deque_pool.push(deque);
}

/// Runs the resolution phase on the graph. The resolution phase is when 4 main pieces of information are computed:
///
/// 1. Declarations for all definitions
Expand Down Expand Up @@ -1037,9 +1081,11 @@ impl<'a> Resolver<'a> {
}

let mut state = ChainState::default();
let parent_ancestors = self.linearize_parent_ancestors(declaration_id, &mut state);
let mut parent_buffer = self.take_chain_buffer();
let has_parent = self.linearize_parent_ancestors_into(declaration_id, &mut state, &mut parent_buffer);

let mut mixins = self.take_mixin_buffer();
let declaration = self.graph.declarations().get(&declaration_id).unwrap();
let mut mixins = Vec::new();

let is_singleton_class = matches!(declaration, Declaration::Namespace(Namespace::SingletonClass(_)));

Expand Down Expand Up @@ -1074,18 +1120,34 @@ impl<'a> Resolver<'a> {
self.get_or_create_singleton_class(declaration_id, SingletonAncestors::Enqueue);
}

let (linearized_prepends, linearized_includes) =
self.linearize_mixins(mixins, parent_ancestors.as_ref(), &mut state);
let mut linearized_prepends = self.take_deque();
let mut linearized_includes = self.take_deque();

self.linearize_mixins(
&mut state,
&mixins,
has_parent.then_some(parent_buffer.as_slice()),
&mut linearized_prepends,
&mut linearized_includes,
);

// Build the final list
let mut ancestors = Vec::new();
ancestors.extend(linearized_prepends);
// Build the final list with its final capacity.
let final_len = linearized_prepends.len() + 1 + linearized_includes.len() + parent_buffer.len();
let mut ancestors = Vec::with_capacity(final_len);
ancestors.extend(linearized_prepends.drain(..));
ancestors.push(Ancestor::Complete(declaration_id));
ancestors.extend(linearized_includes);
if let Some(parents) = parent_ancestors {
ancestors.extend(parents);
ancestors.extend(linearized_includes.drain(..));
if has_parent {
ancestors.extend_from_slice(&parent_buffer);
}

// The scratch buffers gave up their contents to the new list, thus they go back to the pool
// for the next linearization to use.
self.return_chain_buffer(parent_buffer);
self.return_mixin_buffer(mixins);
self.return_deque(linearized_prepends);
self.return_deque(linearized_includes);

let result = if state.cyclic {
Ancestors::Cyclic(ancestors)
} else if state.partial {
Expand All @@ -1106,17 +1168,21 @@ impl<'a> Resolver<'a> {
result
}

fn linearize_parent_ancestors(
/// Copy the parent ancestor chain of a declaration into `buffer`.
///
/// Returns `true` if the declaration has a parent. A module, or a class without a superclass,
/// has no parent and leaves `buffer` untouched.
fn linearize_parent_ancestors_into(
&mut self,
declaration_id: DeclarationId,
state: &mut ChainState,
) -> Option<Vec<Ancestor>> {
buffer: &mut Vec<Ancestor>,
) -> bool {
let declaration = self.graph.declarations().get(&declaration_id).unwrap();

match declaration {
Declaration::Namespace(Namespace::Class(_)) => {
let superclass = self.linearize_superclass(declaration_id, state)?;
Some(state.fold(superclass))
self.linearize_superclass_into(declaration_id, state, buffer)
}
Declaration::Namespace(Namespace::SingletonClass(_)) => {
let owner_id = *declaration.owner_id();
Expand All @@ -1126,24 +1192,29 @@ impl<'a> Resolver<'a> {
state.partial = true;
}

let parent = self.linearize_ancestors(singleton_parent_id);
Some(state.fold(parent))
let result = self.linearize_ancestors(singleton_parent_id);
match &result {
Ancestors::Complete(_) => {}
Ancestors::Cyclic(_) => state.cyclic = true,
Ancestors::Partial(_) => state.partial = true,
}
buffer.extend(result.iter());
true
}
_ => None,
_ => false,
}
}

/// Linearize all mixins into a prepend and include list. This function requires the parent ancestors because included
/// modules are deduplicated against them
fn linearize_mixins(
&mut self,
mixins: Vec<Mixin>,
parent_ancestors: Option<&Vec<Ancestor>>,
state: &mut ChainState,
) -> (VecDeque<Ancestor>, VecDeque<Ancestor>) {
let mut linearized_prepends = VecDeque::new();
let mut linearized_includes = VecDeque::new();

mixins: &[Mixin],
parent_ancestors: Option<&[Ancestor]>,
linearized_prepends: &mut VecDeque<Ancestor>,
linearized_includes: &mut VecDeque<Ancestor>,
) {
// IMPORTANT! In the slice of mixins we receive, extends are the ones that occurred in the attached object, which we
// collect ahead of time. This is the reason why we apparently treat an extend like an include, because an extend in
// the attached object is equivalent to an include in the singleton class
Expand Down Expand Up @@ -1199,9 +1270,7 @@ impl<'a> Resolver<'a> {
ids.retain(|id| {
!linearized_prepends.contains(id)
&& !linearized_includes.contains(id)
&& parent_ancestors
.as_ref()
.is_none_or(|parent_ids| !parent_ids.contains(id))
&& parent_ancestors.is_none_or(|parent_ids| !parent_ids.contains(id))
});

for id in ids.into_iter().rev() {
Expand All @@ -1218,8 +1287,6 @@ impl<'a> Resolver<'a> {
}
}
}

(linearized_prepends, linearized_includes)
}

/// Propagate descendants to all cached ancestors
Expand Down Expand Up @@ -2063,24 +2130,41 @@ impl<'a> Resolver<'a> {
))
}

fn linearize_superclass(&mut self, declaration_id: DeclarationId, state: &mut ChainState) -> Option<Ancestors> {
let (superclass_id, unresolved_superclass) = self.get_superclass(declaration_id)?;
let mut result = self.linearize_ancestors(superclass_id);
/// Copy the linearized superclass chain of a class into `buffer`. Returns `true` if the class
/// has a superclass.
///
/// An unresolved superclass reference means the chain must be partial, never cyclic. A cyclic
/// chain counts as settled (`has_complete_ancestors` is true for `Cyclic`), which would stop
/// retries for the unresolved reference.
fn linearize_superclass_into(
&mut self,
declaration_id: DeclarationId,
state: &mut ChainState,
buffer: &mut Vec<Ancestor>,
) -> bool {
let Some((superclass_id, unresolved_superclass)) = self.get_superclass(declaration_id) else {
return false;
};

let result = self.linearize_ancestors(superclass_id);

if let Some(name_id) = unresolved_superclass {
state.partial = true;

// Insert the unresolved superclass as a Partial ancestor at the front of the chain, so it
// Put the unresolved superclass as a Partial ancestor at the front of the chain, so it
// appears before the default Object ancestors
let ancestors = match &mut result {
Ancestors::Complete(ids) | Ancestors::Cyclic(ids) | Ancestors::Partial(ids) => ids,
};
ancestors.insert(0, Ancestor::Partial(name_id));

result = result.to_partial();
buffer.push(Ancestor::Partial(name_id));
buffer.extend(result.iter());
} else {
match &result {
Ancestors::Complete(_) => {}
Ancestors::Cyclic(_) => state.cyclic = true,
Ancestors::Partial(_) => state.partial = true,
}
buffer.extend(result.iter());
}

Some(result)
true
}

fn mixins_of(&self, definition_id: DefinitionId) -> &[Mixin] {
Expand Down
Loading