From 3d3f26a63a7e16d4a6371294bc7543ffa9a2024b Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Mon, 11 Mar 2024 17:41:18 +0000 Subject: [PATCH 1/5] make a test case --- src/internal/core.rs | 9 +++++++ tests/tests.rs | 61 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/internal/core.rs b/src/internal/core.rs index fa986f34..8b21fea6 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -131,7 +131,16 @@ impl State { break; } Relation::AlmostSatisfied(package_almost) => { + assert!( + self.unit_propagation_buffer + .iter() + .filter(|p| p == &&package_almost) + .count() + <= 10, + ); + // if !self.unit_propagation_buffer.contains(&package_almost) { self.unit_propagation_buffer.push(package_almost.clone()); + // } // Add (not term) to the partial solution with incompat as cause. self.partial_solution.add_derivation( package_almost, diff --git a/tests/tests.rs b/tests/tests.rs index 81fd5324..79ec3ea8 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -54,3 +54,64 @@ fn cannot_depend_on_self() { Err(PubGrubError::SelfDependency { .. }) )); } + +#[test] +fn redundant_scanning() { + let mut dependency_provider = OfflineDependencyProvider::::new(); + dependency_provider.add_dependencies( + 0, + 0, + [ + (247, Range::higher_than(1)), + (71, Range::strictly_lower_than(7)), + (69, Range::higher_than(4)), + (1, Range::full()), + ], + ); + + dependency_provider.add_dependencies(1, 0, [(646, Range::singleton(0))]); + dependency_provider.add_dependencies(1, 1, [(247, Range::strictly_lower_than(3))]); + dependency_provider.add_dependencies(1, 2, [(71, Range::higher_than(7))]); + dependency_provider.add_dependencies(1, 3, [(66, Range::full())]); + dependency_provider.add_dependencies(1, 4, [(2, Range::empty())]); + dependency_provider.add_dependencies(1, 5, [(3, Range::empty())]); + dependency_provider.add_dependencies(1, 6, [(248, Range::from_range_bounds(1..3))]); + dependency_provider.add_dependencies(1, 7, [(247, Range::singleton(0))]); + dependency_provider.add_dependencies(1, 8, [(247, Range::singleton(1))]); + dependency_provider.add_dependencies(1, 9, [(248, Range::singleton(1))]); + dependency_provider.add_dependencies(1, 10, [(71, Range::singleton(7))]); + dependency_provider.add_dependencies(1, 11, [(99, Range::full())]); + dependency_provider.add_dependencies(1, 12, [(69, Range::strictly_lower_than(6))]); + + dependency_provider.add_dependencies(66, 0, [(69, Range::strictly_lower_than(5))]); + + dependency_provider.add_dependencies( + 69, + 7, + [(176, Range::singleton(3)), (248, Range::singleton(0))], + ); + dependency_provider.add_dependencies( + 69, + 10, + [(248, Range::singleton(0)), (646, Range::singleton(3))], + ); + + dependency_provider.add_dependencies(99, 0, [(248, Range::singleton(1))]); + + dependency_provider.add_dependencies(71, 0, []); + dependency_provider.add_dependencies(71, 2, []); + dependency_provider.add_dependencies(71, 3, []); + dependency_provider.add_dependencies(71, 4, []); + dependency_provider.add_dependencies(71, 5, []); + + dependency_provider.add_dependencies(247, 3, []); + dependency_provider.add_dependencies(247, 6, [(249, Range::empty())]); + + dependency_provider.add_dependencies(248, 0, []); + dependency_provider.add_dependencies(248, 1, []); + + dependency_provider.add_dependencies(646, 0, []); + dependency_provider.add_dependencies(646, 3, []); + + _ = resolve(&dependency_provider, 0, 0); +} From 1fe9afc12edb2cb3aa34e588a098f990303403f6 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Mon, 11 Mar 2024 17:41:18 +0000 Subject: [PATCH 2/5] do not add things to the buffer more than once --- src/internal/core.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/internal/core.rs b/src/internal/core.rs index 8b21fea6..34733aa6 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -131,16 +131,9 @@ impl State { break; } Relation::AlmostSatisfied(package_almost) => { - assert!( - self.unit_propagation_buffer - .iter() - .filter(|p| p == &&package_almost) - .count() - <= 10, - ); - // if !self.unit_propagation_buffer.contains(&package_almost) { - self.unit_propagation_buffer.push(package_almost.clone()); - // } + if !self.unit_propagation_buffer.contains(&package_almost) { + self.unit_propagation_buffer.push(package_almost.clone()); + } // Add (not term) to the partial solution with incompat as cause. self.partial_solution.add_derivation( package_almost, From c0ebc64e453752948c9ac120d2ffdf0832eb1333 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Mon, 11 Mar 2024 17:41:18 +0000 Subject: [PATCH 3/5] remove useless test --- tests/tests.rs | 61 -------------------------------------------------- 1 file changed, 61 deletions(-) diff --git a/tests/tests.rs b/tests/tests.rs index 79ec3ea8..81fd5324 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -54,64 +54,3 @@ fn cannot_depend_on_self() { Err(PubGrubError::SelfDependency { .. }) )); } - -#[test] -fn redundant_scanning() { - let mut dependency_provider = OfflineDependencyProvider::::new(); - dependency_provider.add_dependencies( - 0, - 0, - [ - (247, Range::higher_than(1)), - (71, Range::strictly_lower_than(7)), - (69, Range::higher_than(4)), - (1, Range::full()), - ], - ); - - dependency_provider.add_dependencies(1, 0, [(646, Range::singleton(0))]); - dependency_provider.add_dependencies(1, 1, [(247, Range::strictly_lower_than(3))]); - dependency_provider.add_dependencies(1, 2, [(71, Range::higher_than(7))]); - dependency_provider.add_dependencies(1, 3, [(66, Range::full())]); - dependency_provider.add_dependencies(1, 4, [(2, Range::empty())]); - dependency_provider.add_dependencies(1, 5, [(3, Range::empty())]); - dependency_provider.add_dependencies(1, 6, [(248, Range::from_range_bounds(1..3))]); - dependency_provider.add_dependencies(1, 7, [(247, Range::singleton(0))]); - dependency_provider.add_dependencies(1, 8, [(247, Range::singleton(1))]); - dependency_provider.add_dependencies(1, 9, [(248, Range::singleton(1))]); - dependency_provider.add_dependencies(1, 10, [(71, Range::singleton(7))]); - dependency_provider.add_dependencies(1, 11, [(99, Range::full())]); - dependency_provider.add_dependencies(1, 12, [(69, Range::strictly_lower_than(6))]); - - dependency_provider.add_dependencies(66, 0, [(69, Range::strictly_lower_than(5))]); - - dependency_provider.add_dependencies( - 69, - 7, - [(176, Range::singleton(3)), (248, Range::singleton(0))], - ); - dependency_provider.add_dependencies( - 69, - 10, - [(248, Range::singleton(0)), (646, Range::singleton(3))], - ); - - dependency_provider.add_dependencies(99, 0, [(248, Range::singleton(1))]); - - dependency_provider.add_dependencies(71, 0, []); - dependency_provider.add_dependencies(71, 2, []); - dependency_provider.add_dependencies(71, 3, []); - dependency_provider.add_dependencies(71, 4, []); - dependency_provider.add_dependencies(71, 5, []); - - dependency_provider.add_dependencies(247, 3, []); - dependency_provider.add_dependencies(247, 6, [(249, Range::empty())]); - - dependency_provider.add_dependencies(248, 0, []); - dependency_provider.add_dependencies(248, 1, []); - - dependency_provider.add_dependencies(646, 0, []); - dependency_provider.add_dependencies(646, 3, []); - - _ = resolve(&dependency_provider, 0, 0); -} From 03787f9ceaeea1cb4d2acdbceb70f076abf4335f Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Mon, 11 Mar 2024 17:41:18 +0000 Subject: [PATCH 4/5] a cheaper check --- src/internal/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/core.rs b/src/internal/core.rs index 34733aa6..f8ac91bb 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -131,7 +131,7 @@ impl State { break; } Relation::AlmostSatisfied(package_almost) => { - if !self.unit_propagation_buffer.contains(&package_almost) { + if self.unit_propagation_buffer.last() != Some(&package_almost) { self.unit_propagation_buffer.push(package_almost.clone()); } // Add (not term) to the partial solution with incompat as cause. From feb6b55af353a425988082c9373c7eee4ed37479 Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Mon, 11 Mar 2024 17:49:16 +0000 Subject: [PATCH 5/5] add a comment --- src/internal/core.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal/core.rs b/src/internal/core.rs index f8ac91bb..ca598564 100644 --- a/src/internal/core.rs +++ b/src/internal/core.rs @@ -131,6 +131,11 @@ impl State { break; } Relation::AlmostSatisfied(package_almost) => { + // Add `package_almost` to the `unit_propagation_buffer` set. + // Putting items in `unit_propagation_buffer` more than once waste cycles, + // but so does checking for duplicates. + // In practice the most common pathology is adding the same package repeatedly. + // So we only check if it is duplicated with the last item. if self.unit_propagation_buffer.last() != Some(&package_almost) { self.unit_propagation_buffer.push(package_almost.clone()); }