Skip to content

Commit

Permalink
perf: small memory performance optimizations (#35)
Browse files Browse the repository at this point in the history
reduce DecisionAndLevel from 8 to 4 bytes and use `ahash`.
  • Loading branch information
baszalmstra authored Jun 3, 2024
1 parent 9c07c46 commit a72ffd3
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 26 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ readme = "README.md"
resolver = "2"

[dependencies]
ahash = "0.8.11"
itertools = "0.13"
petgraph = "0.6.5"
tracing = "0.1.40"
Expand Down
4 changes: 2 additions & 2 deletions src/internal/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ impl<TId: ArenaId, TValue> Arena<TId, TValue> {
}

/// Returns an iterator over the elements of the arena.
pub fn iter(&self) -> ArenaIter<TId, TValue> {
pub fn iter(&self) -> ArenaIter<'_, TId, TValue> {
ArenaIter {
arena: self,
index: 0,
}
}

/// Returns an mutable iterator over the elements of the arena.
pub fn iter_mut(&mut self) -> ArenaIterMut<TId, TValue> {
pub fn iter_mut(&mut self) -> ArenaIterMut<'_, TId, TValue> {
ArenaIterMut {
arena: self,
index: 0,
Expand Down
3 changes: 1 addition & 2 deletions src/internal/frozen_copy_map.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use std::borrow::Borrow;
use std::cell::UnsafeCell;
use std::collections::hash_map::RandomState;
use std::collections::HashMap;
use std::hash::{BuildHasher, Hash};

/// An insert only map where items can only be returned by cloning the values. This ensures that the
/// map can safely be used in an immutable context.
pub struct FrozenCopyMap<K, V, S = RandomState> {
pub struct FrozenCopyMap<K, V, S = ahash::RandomState> {
map: UnsafeCell<HashMap<K, V, S>>,
}

Expand Down
2 changes: 1 addition & 1 deletion src/internal/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl SolvableId {
pub fn display<VS: VersionSet, N: PackageName + Display>(
self,
pool: &Pool<VS, N>,
) -> DisplaySolvable<VS, N> {
) -> DisplaySolvable<'_, VS, N> {
pool.resolve_internal_solvable(self).display(pool)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/internal/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl<TId: ArenaId, TValue> Mapping<TId, TValue> {
}

/// Returns an iterator over all the existing key value pairs.
pub fn iter(&self) -> MappingIter<TId, TValue> {
pub fn iter(&self) -> MappingIter<'_, TId, TValue> {
MappingIter {
mapping: self,
offset: 0,
Expand Down
6 changes: 3 additions & 3 deletions src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ pub struct Pool<VS: VersionSet, N: PackageName = String> {
package_names: Arena<NameId, N>,

/// Map from package names to the id of their interned counterpart
pub(crate) names_to_ids: FrozenCopyMap<N, NameId>,
pub(crate) names_to_ids: FrozenCopyMap<N, NameId, ahash::RandomState>,

/// Interned strings
strings: Arena<StringId, String>,

/// Map from package names to the id of their interned counterpart
pub(crate) string_to_ids: FrozenCopyMap<String, StringId>,
pub(crate) string_to_ids: FrozenCopyMap<String, StringId, ahash::RandomState>,

/// Interned match specs
pub(crate) version_sets: Arena<VersionSetId, (NameId, VS)>,

/// Map from version set to the id of their interned counterpart
version_set_to_id: FrozenCopyMap<(NameId, VS), VersionSetId>,
version_set_to_id: FrozenCopyMap<(NameId, VS), VersionSetId, ahash::RandomState>,
}

impl<VS: VersionSet, N: PackageName> Default for Pool<VS, N> {
Expand Down
9 changes: 5 additions & 4 deletions src/problem.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Types to examine why a problem was unsatisfiable, and to report the causes to the user.
use std::collections::{HashMap, HashSet};
use ahash::HashMap;
use std::collections::HashSet;
use std::fmt;
use std::fmt::{Display, Formatter};
use std::hash::Hash;
Expand Down Expand Up @@ -277,7 +278,7 @@ impl ProblemGraph {
let merged_nodes = if simplify {
self.simplify(pool)
} else {
HashMap::new()
HashMap::default()
};

write!(f, "digraph {{")?;
Expand Down Expand Up @@ -357,7 +358,7 @@ impl ProblemGraph {
let graph = &self.graph;

// Gather information about nodes that can be merged
let mut maybe_merge = HashMap::new();
let mut maybe_merge = HashMap::default();
for node_id in graph.node_indices() {
let candidate = match graph[node_id] {
ProblemNode::UnresolvedDependency | ProblemNode::Excluded(_) => continue,
Expand Down Expand Up @@ -653,7 +654,7 @@ impl<'pool, VS: VersionSet, N: PackageName + Display, M: SolvableDisplay<VS, N>>
fn fmt_graph(
&self,
f: &mut Formatter<'_>,
top_level_edges: &[EdgeReference<ProblemEdge>],
top_level_edges: &[EdgeReference<'_, ProblemEdge>],
top_level_indent: bool,
) -> fmt::Result
where
Expand Down
10 changes: 6 additions & 4 deletions src/solver/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ use crate::{
Candidates, Dependencies, DependencyProvider, NameId, PackageName, Pool, SolvableId,
VersionSet, VersionSetId,
};
use ahash::HashMap;
use bitvec::vec::BitVec;
use elsa::FrozenMap;
use event_listener::Event;
use std::{any::Any, cell::RefCell, collections::HashMap, marker::PhantomData, rc::Rc};
use std::{any::Any, cell::RefCell, marker::PhantomData, rc::Rc};

/// Keeps a cache of previously computed and/or requested information about solvables and version
/// sets.
Expand All @@ -24,14 +25,15 @@ pub struct SolverCache<VS: VersionSet, N: PackageName, D: DependencyProvider<VS,
package_name_to_candidates_in_flight: RefCell<HashMap<NameId, Rc<Event>>>,

/// A mapping of `VersionSetId` to the candidates that match that set.
version_set_candidates: FrozenMap<VersionSetId, Vec<SolvableId>>,
version_set_candidates: FrozenMap<VersionSetId, Vec<SolvableId>, ahash::RandomState>,

/// A mapping of `VersionSetId` to the candidates that do not match that set (only candidates
/// of the package indicated by the version set are included).
version_set_inverse_candidates: FrozenMap<VersionSetId, Vec<SolvableId>>,
version_set_inverse_candidates: FrozenMap<VersionSetId, Vec<SolvableId>, ahash::RandomState>,

/// A mapping of `VersionSetId` to a sorted list of candidates that match that set.
pub(crate) version_set_to_sorted_candidates: FrozenMap<VersionSetId, Vec<SolvableId>>,
pub(crate) version_set_to_sorted_candidates:
FrozenMap<VersionSetId, Vec<SolvableId>, ahash::RandomState>,

/// A mapping from a solvable to a list of dependencies
solvable_dependencies: Arena<DependenciesId, Dependencies>,
Expand Down
12 changes: 10 additions & 2 deletions src/solver/clause.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,11 @@ impl Clause {
pub fn visit_literals(
&self,
learnt_clauses: &Arena<LearntClauseId, Vec<Literal>>,
version_set_to_sorted_candidates: &FrozenMap<VersionSetId, Vec<SolvableId>>,
version_set_to_sorted_candidates: &FrozenMap<
VersionSetId,
Vec<SolvableId>,
ahash::RandomState,
>,
mut visit: impl FnMut(Literal),
) {
match *self {
Expand Down Expand Up @@ -487,7 +491,11 @@ impl ClauseState {
pub fn next_unwatched_variable(
&self,
learnt_clauses: &Arena<LearntClauseId, Vec<Literal>>,
version_set_to_sorted_candidates: &FrozenMap<VersionSetId, Vec<SolvableId>>,
version_set_to_sorted_candidates: &FrozenMap<
VersionSetId,
Vec<SolvableId>,
ahash::RandomState,
>,
decision_map: &DecisionMap,
) -> Option<SolvableId> {
// The next unwatched variable (if available), is a variable that is:
Expand Down
7 changes: 4 additions & 3 deletions src/solver/decision_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::fmt::{Display, Formatter};
/// < 0: level of decision when the solvable is set to false
#[repr(transparent)]
#[derive(Copy, Clone)]
struct DecisionAndLevel(i64);
struct DecisionAndLevel(i32);

impl DecisionAndLevel {
fn undecided() -> DecisionAndLevel {
Expand All @@ -26,11 +26,12 @@ impl DecisionAndLevel {
}

fn level(self) -> u32 {
self.0.unsigned_abs() as u32
self.0.unsigned_abs()
}

fn with_value_and_level(value: bool, level: u32) -> Self {
Self(if value { level as i64 } else { -(level as i64) })
debug_assert!(level <= (i32::MAX as u32), "level is too large");
Self(if value { level as i32 } else { -(level as i32) })
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/solver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1019,9 +1019,10 @@ impl<VS: VersionSet, N: PackageName + Display, D: DependencyProvider<VS, N>, RT:
let mut predecessor_clause_id: Option<ClauseId> = None;
let mut clause_id = self.watches.first_clause_watching_solvable(pkg);
while !clause_id.is_null() {
if predecessor_clause_id == Some(clause_id) {
panic!("Linked list is circular!");
}
debug_assert!(
predecessor_clause_id != Some(clause_id),
"Linked list is circular!"
);

// Get mutable access to both clauses.
let mut clauses = self.clauses.borrow_mut();
Expand Down
3 changes: 2 additions & 1 deletion tests/solver.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
any::Any,
cell::{Cell, RefCell},
collections::{HashMap, HashSet},
collections::HashSet,
fmt::{Debug, Display, Formatter},
io::{stderr, Write},
num::ParseIntError,
Expand All @@ -14,6 +14,7 @@ use std::{
time::Duration,
};

use ahash::HashMap;
use indexmap::IndexMap;
use itertools::Itertools;
use resolvo::{
Expand Down

0 comments on commit a72ffd3

Please sign in to comment.