avoid cycles during serialization

Author: ibraheemdev

src/accumulator.rs

@@ -8,7 +8,7 @@ use std::panic::UnwindSafe;
 use accumulated::{Accumulated, AnyAccumulated};
 
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::ingredient::{Ingredient, Jar};
 use crate::plumbing::ZalsaLocal;
 use crate::sync::Arc;
@@ -117,6 +117,7 @@ impl<A: Accumulator> Ingredient for IngredientImpl<A> {
         _zalsa: &Zalsa,
         _edge: QueryEdge,
         _serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         panic!("nothing should ever depend on an accumulator directly")
     }

src/function.rs

@@ -13,7 +13,7 @@ use crate::cycle::{
 use crate::database::RawDatabase;
 use crate::function::delete::DeletedEntries;
 use crate::function::sync::{ClaimResult, SyncTable};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::ingredient::{Ingredient, WaitForResult};
 use crate::key::DatabaseKeyIndex;
 use crate::plumbing::{self, MemoIngredientMap};
@@ -294,6 +294,7 @@ where
         zalsa: &Zalsa,
         edge: QueryEdge,
         serialized_edges: &mut FxIndexSet<QueryEdge>,
+        visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         let input = edge.key().key_index();
 
@@ -305,10 +306,27 @@ where
 
         let origin = memo.revisions.origin.as_ref();
 
+        visited_edges.insert(edge);
+
         // Collect the minimum dependency tree.
         for edge in origin.edges() {
+            // Avoid forming cycles.
+            if visited_edges.contains(edge) {
+                continue;
+            }
+
+            // Avoid flattening edges that we're going to serialize directly.
+            if serialized_edges.contains(edge) {
+                continue;
+            }
+
             let dependency = zalsa.lookup_ingredient(edge.key().ingredient_index());
-            dependency.collect_minimum_serialized_edges(zalsa, *edge, serialized_edges)
+            dependency.collect_minimum_serialized_edges(
+                zalsa,
+                *edge,
+                serialized_edges,
+                visited_edges,
+            )
         }
     }
 
@@ -494,7 +512,7 @@ where
 #[cfg(feature = "persistence")]
 mod persistence {
     use super::{Configuration, IngredientImpl, Memo};
-    use crate::hash::FxIndexSet;
+    use crate::hash::{FxHashSet, FxIndexSet};
     use crate::plumbing::{MemoIngredientMap, SalsaStructInDb};
     use crate::zalsa::Zalsa;
     use crate::zalsa_local::{QueryEdge, QueryOrigin, QueryOriginRef};
@@ -579,6 +597,7 @@ mod persistence {
 
     // Flatten the dependency edges before serialization.
     fn flatten_edges(zalsa: &Zalsa, edges: &[QueryEdge]) -> FxIndexSet<QueryEdge> {
+        let mut visited_edges = FxIndexSet::default();
         let mut flattened_edges =
             FxIndexSet::with_capacity_and_hasher(edges.len(), Default::default());
 
@@ -590,7 +609,12 @@ mod persistence {
                 flattened_edges.insert(edge);
             } else {
                 // Otherwise, serialize the minimum edges necessary to cover the dependency.
-                dependency.collect_minimum_serialized_edges(zalsa, edge, &mut flattened_edges);
+                dependency.collect_minimum_serialized_edges(
+                    zalsa,
+                    edge,
+                    &mut flattened_edges,
+                    &mut visited_edges,
+                );
             }
         }
 

src/ingredient.rs

@@ -6,7 +6,7 @@ use crate::cycle::{
 };
 use crate::database::RawDatabase;
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::runtime::Running;
 use crate::sync::Arc;
 use crate::table::memo::MemoTableTypes;
@@ -68,6 +68,7 @@ pub trait Ingredient: Any + std::fmt::Debug + Send + Sync {
         zalsa: &Zalsa,
         edge: QueryEdge,
         serialized_edges: &mut FxIndexSet<QueryEdge>,
+        visited_edges: &mut FxHashSet<QueryEdge>,
     );
 
     /// Returns information about the current provisional status of `input`.

src/input.rs

@@ -9,7 +9,7 @@ pub mod singleton;
 use input_field::FieldIngredientImpl;
 
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::id::{AsId, FromId, FromIdWithDb};
 use crate::ingredient::Ingredient;
 use crate::input::singleton::{Singleton, SingletonChoice};
@@ -284,6 +284,7 @@ impl<C: Configuration> Ingredient for IngredientImpl<C> {
         _zalsa: &Zalsa,
         _edge: QueryEdge,
         _serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         panic!("nothing should ever depend on an input struct directly")
     }

src/input/input_field.rs

@@ -2,7 +2,7 @@ use std::fmt;
 use std::marker::PhantomData;
 
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::ingredient::Ingredient;
 use crate::input::{Configuration, IngredientImpl, Value};
 use crate::sync::Arc;
@@ -68,10 +68,12 @@ where
         _zalsa: &Zalsa,
         edge: QueryEdge,
         serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         assert!(
             C::PERSIST,
-            "the inputs of a persistable tracked function must be persistable"
+            "the inputs of a persistable tracked function must be persistable: `{}` is not persistable",
+            C::DEBUG_NAME
         );
 
         // Input dependencies are the leaves of the minimum dependency tree.

src/interned.rs

@@ -12,7 +12,7 @@ use rustc_hash::FxBuildHasher;
 
 use crate::durability::Durability;
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::id::{AsId, FromId};
 use crate::ingredient::Ingredient;
 use crate::plumbing::{self, Jar, ZalsaLocal};
@@ -903,6 +903,7 @@ where
         _zalsa: &Zalsa,
         edge: QueryEdge,
         serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         if C::PERSIST {
             // If the interned struct is being persisted, it may be reachable through transitive queries.

src/tracked_struct.rs

@@ -12,7 +12,7 @@ use thin_vec::ThinVec;
 use tracked_field::FieldIngredientImpl;
 
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::id::{AsId, FromId};
 use crate::ingredient::{Ingredient, Jar};
 use crate::key::DatabaseKeyIndex;
@@ -949,6 +949,7 @@ where
         _zalsa: &Zalsa,
         _edge: QueryEdge,
         _serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         // Note that tracked structs are referenced by the identity map, but that
         // only matters if we are serializing the creating query, in which case

src/tracked_struct/tracked_field.rs

@@ -1,7 +1,7 @@
 use std::marker::PhantomData;
 
 use crate::function::{VerifyCycleHeads, VerifyResult};
-use crate::hash::FxIndexSet;
+use crate::hash::{FxHashSet, FxIndexSet};
 use crate::ingredient::Ingredient;
 use crate::sync::Arc;
 use crate::table::memo::MemoTableTypes;
@@ -74,6 +74,7 @@ where
         _zalsa: &Zalsa,
         _edge: QueryEdge,
         _serialized_edges: &mut FxIndexSet<QueryEdge>,
+        _visited_edges: &mut FxHashSet<QueryEdge>,
     ) {
         // Tracked fields do not have transitive dependencies, and their dependencies are covered by
         // the base inputs.