Skip to content

Commit

Permalink
perf: visit fewer clauses during propagation (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
baszalmstra authored Sep 30, 2024
1 parent e1d5665 commit 620f364
Show file tree
Hide file tree
Showing 17 changed files with 648 additions and 622 deletions.
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,12 @@ bitvec = "1.0.1"
serde = { version = "1.0", features = ["derive"], optional = true }
futures = { version = "0.3", default-features = false, features = ["alloc"] }
event-listener = "5.3"

indexmap = "2"
tokio = { version = "1.37", features = ["rt"], optional = true }
async-std = { version = "1.12", default-features = false, features = ["alloc", "default"], optional = true }

[dev-dependencies]
insta = "1.39.0"
indexmap = "2"
proptest = "1.2"
tracing-test = { version = "0.2.4", features = ["no-env-filter"] }
tokio = { version = "1.35.1", features = ["time", "rt"] }
Expand Down
9 changes: 5 additions & 4 deletions src/conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use petgraph::{
Direction,
};

use crate::internal::arena::ArenaId;
use crate::{
internal::id::{ClauseId, InternalSolvableId, SolvableId, StringId, VersionSetId},
runtime::AsyncRuntime,
Expand Down Expand Up @@ -52,11 +53,11 @@ impl Conflict {
let unresolved_node = graph.add_node(ConflictNode::UnresolvedDependency);

for clause_id in &self.clauses {
let clause = &solver.clauses.borrow()[*clause_id].kind;
let clause = &solver.clauses.kinds.borrow()[clause_id.to_usize()];
match clause {
Clause::InstallRoot => (),
Clause::Excluded(solvable, reason) => {
tracing::info!("{solvable:?} is excluded");
tracing::trace!("{solvable:?} is excluded");
let package_node = Self::add_node(&mut graph, &mut nodes, *solvable);
let excluded_node = excluded_nodes
.entry(*reason)
Expand All @@ -76,7 +77,7 @@ impl Conflict {
unreachable!("The version set was used in the solver, so it must have been cached. Therefore cancellation is impossible here and we cannot get an `Err(...)`")
});
if candidates.is_empty() {
tracing::info!(
tracing::trace!(
"{package_id:?} requires {version_set_id:?}, which has no candidates"
);
graph.add_edge(
Expand All @@ -86,7 +87,7 @@ impl Conflict {
);
} else {
for &candidate_id in candidates {
tracing::info!("{package_id:?} requires {candidate_id:?}");
tracing::trace!("{package_id:?} requires {candidate_id:?}");

let candidate_node =
Self::add_node(&mut graph, &mut nodes, candidate_id.into());
Expand Down
6 changes: 3 additions & 3 deletions src/internal/arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ impl<TId: ArenaId, TValue> Arena<TId, TValue> {
pub fn get_two_mut(&mut self, a: TId, b: TId) -> (&mut TValue, &mut TValue) {
let a_index = a.to_usize();
let b_index = b.to_usize();
assert!(a_index < self.len());
assert!(b_index < self.len());
assert_ne!(a_index, b_index);
debug_assert!(a_index < self.len());
debug_assert!(b_index < self.len());
debug_assert_ne!(a_index, b_index);
let (a_chunk, a_offset) = Self::chunk_and_offset(a_index);
let (b_chunk, b_offset) = Self::chunk_and_offset(b_index);
// SAFE: because we check that the indices are less than the length and that both indices do
Expand Down
17 changes: 3 additions & 14 deletions src/internal/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,10 @@ impl ArenaId for VersionSetUnionId {
#[cfg_attr(feature = "serde", serde(transparent))]
pub struct SolvableId(pub u32);

/// Internally used id for solvables that can also represent root and null.
/// Internally used id for solvables that can also represent the root.
#[repr(transparent)]
#[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, Ord, PartialOrd)]
pub(crate) struct InternalSolvableId(u32);

const INTERNAL_SOLVABLE_NULL: u32 = u32::MAX;
pub(crate) struct InternalSolvableId(pub u32);
const INTERNAL_SOLVABLE_ROOT: u32 = 0;

impl InternalSolvableId {
Expand All @@ -97,17 +95,9 @@ impl InternalSolvableId {
self.0 == 0
}

pub const fn null() -> Self {
Self(u32::MAX)
}

pub const fn is_null(self) -> bool {
self.0 == u32::MAX
}

pub const fn as_solvable(self) -> Option<SolvableId> {
match self.0 {
INTERNAL_SOLVABLE_NULL | INTERNAL_SOLVABLE_ROOT => None,
INTERNAL_SOLVABLE_ROOT => None,
x => Some(SolvableId(x - 1)),
}
}
Expand All @@ -126,7 +116,6 @@ impl<'i, I: Interner> Display for DisplayInternalSolvable<'i, I> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self.id.0 {
INTERNAL_SOLVABLE_ROOT => write!(f, "<root>"),
INTERNAL_SOLVABLE_NULL => write!(f, "<null>"),
x => {
write!(f, "{}", self.interner.display_solvable(SolvableId(x - 1)))
}
Expand Down
5 changes: 3 additions & 2 deletions src/internal/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<TId: ArenaId, TValue> Mapping<TId, TValue> {
}

/// Insert into the mapping with the specific value
pub fn insert(&mut self, id: TId, value: TValue) {
pub fn insert(&mut self, id: TId, value: TValue) -> Option<TValue> {
let idx = id.to_usize();
let (chunk, offset) = Self::chunk_and_offset(idx);

Expand All @@ -59,9 +59,10 @@ impl<TId: ArenaId, TValue> Mapping<TId, TValue> {
self.chunks
.resize_with(chunk + 1, || std::array::from_fn(|_| None));
}
self.chunks[chunk][offset] = Some(value);
let previous_value = self.chunks[chunk][offset].replace(value);
self.len += 1;
self.max = self.max.max(idx);
previous_value
}

/// Get a specific value in the mapping with bound checks
Expand Down
Loading

0 comments on commit 620f364

Please sign in to comment.