Skip to content

Commit

Permalink
Do not modify the subgraph of the target of an Update::NewEdge when t…
Browse files Browse the repository at this point in the history
…he edge target already exists

The scenario:

  * `detect_confilcts_and_updates` walks the `onto` graph as its basis
    for detecting things.

  * If we don't find any candidates for equivalency for a particular
    node in `onto` from the nodes in `to_rebase`, then we do nothing,
    and prune the walk of the `onto` graph at that point.

  * When processing a container in
    `find_container_membership_conflicts_and_updates` (processing a/the
    parent of the potentially new node).

  * There is an entry in `only_onto_edges` for the edge pointing to a
    node. (May or may not be to a new node)

      * We generate an `Update::NewEdge`

  * If the node exists already in `to_rebase` (not new)

    * When processing the DFS event for the node, we continue walking
      the `onto` graph through this node as long as we continue to find
      equivalent nodes in `to_rebase`, generating `Update`s as
      necessary.

  * If the node does _not_ already exist in `to_rebase` (it's new)

    * When processing the DFS event for the node, we prune the `onto`
      graph walk as we could not find an equivalent node, and do not
      generate any `Update` for the node that only exists in `onto`.

So, when `Update::NewEdge` happens in `perform_updates`, if an
equivalent node is found, we want to use it as-is, since there should be
other `Update` to process that will handle any updates necessary for the
target (and any children). If no equivalent node is found, then we want
to import the "new" bits from the subgraph starting at the target of the
edge, since there won't be any `Update` to process that will handle
bringing in those new things.
  • Loading branch information
jhelwig committed Jun 7, 2024
1 parent 182807f commit 7a2aae1
Showing 1 changed file with 4 additions and 16 deletions.
20 changes: 4 additions & 16 deletions lib/dal/src/workspace_snapshot/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ impl WorkspaceSnapshotGraph {
} => {
let updated_source = self.get_latest_node_idx(source.index)?;
let destination =
self.find_in_self_or_create_using_onto(destination.index, onto)?;
self.reuse_from_self_or_import_subgraph_from_onto(destination.index, onto)?;

self.add_edge(updated_source, edge_weight.clone(), destination)?;
}
Expand Down Expand Up @@ -2213,8 +2213,8 @@ impl WorkspaceSnapshotGraph {
Ok(self.get_node_index_by_id(other_id).ok())
}

/// Find in self where self is the "to rebase" side or create using "onto".
fn find_in_self_or_create_using_onto(
/// Find in self (and use it as-is) where self is the "to rebase" side or import the entire subgraph from "onto".
fn reuse_from_self_or_import_subgraph_from_onto(
&mut self,
unchecked: NodeIndex,
onto: &WorkspaceSnapshotGraph,
Expand All @@ -2234,19 +2234,7 @@ impl WorkspaceSnapshotGraph {
};

match equivalent_node {
Some(found_equivalent_node) => {
let found_equivalent_node_weight =
self.get_node_weight(found_equivalent_node)?;
if found_equivalent_node_weight.merkle_tree_hash()
!= unchecked_node_weight.merkle_tree_hash()
{
self.import_subgraph(onto, unchecked)?;
self.find_latest_idx_in_self_from_other_idx(onto, unchecked)?
.ok_or(WorkspaceSnapshotGraphError::NodeWeightNotFound)?
} else {
found_equivalent_node
}
}
Some(found_equivalent_node) => found_equivalent_node,
None => {
self.import_subgraph(onto, unchecked)?;
self.find_latest_idx_in_self_from_other_idx(onto, unchecked)?
Expand Down

0 comments on commit 7a2aae1

Please sign in to comment.