diff --git a/core/src/main/scala/flatgraph/DiffGraphApplier.scala b/core/src/main/scala/flatgraph/DiffGraphApplier.scala index d6f28c69..37d4da74 100644 --- a/core/src/main/scala/flatgraph/DiffGraphApplier.scala +++ b/core/src/main/scala/flatgraph/DiffGraphApplier.scala @@ -407,8 +407,12 @@ private[flatgraph] class DiffGraphApplier(graph: Graph, diff: DiffGraphBuilder) val size = graph.neighbors(pos + 1).asInstanceOf[Array[GNode]].size val oldQty = graph.neighbors(pos).asInstanceOf[Array[Int]] val edgeProp = graph.neighbors(pos + 2) match { - case _: DefaultValue => graph.schema.allocateEdgeProperty(nodeKind, direction, edgeKind, size) - case other => other + case _: DefaultValue => + graph.schema.allocateEdgeProperty(nodeKind, direction, edgeKind, size) + case other => + other + // ^ change this once we switch away from full copy-on-write, see e.g. + // https://github.com/joernio/flatgraph/pull/163#discussion_r1537246314 } val propview = mutable.ArraySeq.make(edgeProp.asInstanceOf[Array[_]]).asInstanceOf[mutable.ArraySeq[Any]] // this will fail if the edge doesn't support properties. todo: better error message diff --git a/core/src/main/scala/flatgraph/Graph.scala b/core/src/main/scala/flatgraph/Graph.scala index 5672f3ec..a4d26dc7 100644 --- a/core/src/main/scala/flatgraph/Graph.scala +++ b/core/src/main/scala/flatgraph/Graph.scala @@ -10,6 +10,8 @@ import java.nio.file.{Files, Path} import java.util.concurrent.atomic.AtomicReferenceArray import org.slf4j.LoggerFactory +import java.util + object Graph { // Slot size is 3 because we have one pointer to array of quantity array and one pointer to array of // neighbors, and one array containing edge properties @@ -33,7 +35,6 @@ object Graph { Graph(schema, storagePathMaybe) } } - } class Graph(val schema: Schema, val storagePathMaybe: Option[Path] = None) extends AutoCloseable { @@ -44,16 +45,16 @@ class Graph(val schema: Schema, val storagePathMaybe: Option[Path] = None) exten private[flatgraph] val livingNodeCountByKind: Array[Int] = new Array[Int](nodeKindCount) - /** Note: this included `deleted` nodes! You might want to use `livingNodeCountByKind` instead. */ - private[flatgraph] def nodeCountByKind(kind: Int): Int = - if (nodesArray.length <= kind) 0 - else nodesArray(kind).length - private[flatgraph] val properties = new Array[AnyRef](nodeKindCount * propertiesCount * PropertySlotSize) private[flatgraph] val inverseIndices = new AtomicReferenceArray[Object](nodeKindCount * propertiesCount * PropertySlotSize) private[flatgraph] val nodesArray: Array[Array[GNode]] = makeNodesArray() private[flatgraph] val neighbors: Array[AnyRef] = makeNeighbors() + /** Note: this included `deleted` nodes! You might want to use `livingNodeCountByKind` instead. */ + private[flatgraph] def nodeCountByKind(kind: Int): Int = + if (nodesArray.length <= kind) 0 + else nodesArray(kind).length + def _nodes(nodeKind: Int): InitNodeIterator[GNode] = { if (nodeKind < 0 || schema.getNumberOfNodeKinds <= nodeKind) InitNodeIteratorArray(Array.empty[GNode]) else if (nodeCountByKind(nodeKind) == livingNodeCountByKind(nodeKind)) new InitNodeIteratorArray[GNode](nodesArray(nodeKind)) diff --git a/core/src/main/scala/flatgraph/misc/TestUtils.scala b/core/src/main/scala/flatgraph/misc/TestUtils.scala index b5031334..e55c8d56 100644 --- a/core/src/main/scala/flatgraph/misc/TestUtils.scala +++ b/core/src/main/scala/flatgraph/misc/TestUtils.scala @@ -2,6 +2,8 @@ package flatgraph.misc import flatgraph.* +import java.nio.file.Path + /** Convenience functions for common use test use-cases. Context: applying a Diff */ object TestUtils { @@ -23,6 +25,42 @@ object TestUtils { applyDiff(_.addNode(node)) node.storedRef.get } + + def copy(storagePathMaybe: Option[Path] = None): Graph = { + val schema = graph.schema + val newGraph = new Graph(schema, storagePathMaybe) + + for (kind <- Range(0, schema.getNumberOfNodeKinds)) { + newGraph.nodesArray(kind) = graph.nodesArray(kind).clone() + newGraph.nodesArray(kind).mapInPlace { oldNode => + val newNode = schema.makeNode(newGraph, oldNode.nodeKind, oldNode.seq()) + if (AccessHelpers.isDeleted(oldNode)) AccessHelpers.markDeleted(newNode) + newNode + } + } + + def cloneThing(item: AnyRef): AnyRef = item match { + case null => null + case a: Array[GNode] => + a.clone().mapInPlace { oldNode => + if (oldNode == null) null + else newGraph.nodesArray(oldNode.nodeKind)(oldNode.seq()) + } + case a: Array[_] => + a.clone() + case other => + other + } + // adjust that once we do diffgraph applications in-place + for (idx <- Range(0, graph.properties.length)) + newGraph.properties(idx) = cloneThing(graph.properties(idx)) + for (idx <- Range(0, graph.neighbors.length)) + newGraph.neighbors(idx) = cloneThing(graph.neighbors(idx)) + System.arraycopy(graph.livingNodeCountByKind, 0, newGraph.livingNodeCountByKind, 0, graph.livingNodeCountByKind.length) + + newGraph + } + } } diff --git a/core/src/test/scala/flatgraph/GraphTests.scala b/core/src/test/scala/flatgraph/GraphTests.scala index 83767393..5965543c 100644 --- a/core/src/test/scala/flatgraph/GraphTests.scala +++ b/core/src/test/scala/flatgraph/GraphTests.scala @@ -6,6 +6,7 @@ import flatgraph.misc.DebugDump.debugDump import flatgraph.storage.Deserialization import flatgraph.storage.Deserialization.DeserializationException import flatgraph.traversal.Language.* +import flatgraph.util.DiffToolTests import org.scalatest.matchers.should.Matchers import org.scalatest.matchers.should.Matchers.shouldBe import org.scalatest.wordspec.AnyWordSpec @@ -1000,4 +1001,43 @@ class GraphTests extends AnyWordSpec with Matchers { } } } + + "copying a graph" in { + import flatgraph.misc.TestUtils.copy + + val graph = DiffToolTests.makeSampleGraph() + val debugDumpOriginalGraph = debugDump(graph) + debugDumpOriginalGraph shouldBe + """#Node numbers (kindId, nnodes) (0: 2), total 2 + |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 1 [dense], 1 [dense]), + | V0_0 : 0: [A], 1: [40] + | V0_0 [0] -> (edgePropertyValue) V0_1 + | V0_1 : 0: [X, Y], 1: [50, 51] + | V0_1 [0] <- (edgePropertyValue) V0_0 + |""".stripMargin + + val graphCopy = graph.copy() + debugDump(graph) shouldBe debugDump(graphCopy) + + // make some changes only to the graph copy + val v0 = graphCopy.node(kind = 0, seq = 0) + val edge = Accessors.getEdgesOut(v0).head + DiffGraphApplier.applyDiff( + graphCopy, + new DiffGraphBuilder(DiffToolTests.sampleSchema) + .setNodeProperty(v0, "0", "updatedNodeProperty") + .setEdgeProperty(edge, "updatedEdgeProperty") + ) + debugDump(graphCopy) shouldBe + """#Node numbers (kindId, nnodes) (0: 2), total 2 + |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 1 [dense], 1 [dense]), + | V0_0 : 0: [updatedNodeProperty], 1: [40] + | V0_0 [0] -> (updatedEdgeProperty) V0_1 + | V0_1 : 0: [X, Y], 1: [50, 51] + | V0_1 [0] <- (updatedEdgeProperty) V0_0 + |""".stripMargin + + // original graph should be untouched + debugDump(graph) shouldBe debugDumpOriginalGraph + } } diff --git a/core/src/test/scala/flatgraph/TestHelpers.scala b/core/src/test/scala/flatgraph/TestHelpers.scala index 40d67470..060a702a 100644 --- a/core/src/test/scala/flatgraph/TestHelpers.scala +++ b/core/src/test/scala/flatgraph/TestHelpers.scala @@ -3,6 +3,7 @@ package flatgraph import flatgraph.TestHelpers.withTemporaryFile import flatgraph.misc.DebugDump.debugDump import flatgraph.misc.Conversions.toShortSafely +import flatgraph.misc.TestUtils.copy import flatgraph.storage.{Deserialization, Serialization} import org.scalatest.matchers.should.Matchers.shouldBe @@ -49,6 +50,10 @@ object TestSchema { val deserialized = Deserialization.readGraph(storagePath, Option(graph.schema)) val newDump = debugDump(deserialized) originalDump shouldBe newDump + + val cloned = graph.copy() + val cloneDump = debugDump(cloned) + originalDump shouldBe cloneDump } } diff --git a/core/src/test/scala/flatgraph/util/DiffToolTests.scala b/core/src/test/scala/flatgraph/util/DiffToolTests.scala index 5e70f2bf..7b328682 100644 --- a/core/src/test/scala/flatgraph/util/DiffToolTests.scala +++ b/core/src/test/scala/flatgraph/util/DiffToolTests.scala @@ -9,6 +9,7 @@ import org.scalatest.wordspec.AnyWordSpec import scala.jdk.CollectionConverters.* class DiffToolTests extends AnyWordSpec { + import DiffToolTests.makeSampleGraph "detects zero changes for the same (identity) graph" in { val graph = makeSampleGraph() @@ -88,29 +89,33 @@ class DiffToolTests extends AnyWordSpec { diff should contain("node flatgraph.GNode[label=V0; id=0] only exists in graph1") } - private def makeSampleGraph(): Graph = { - val schema = TestSchema.make( - nodeKinds = 1, - edgeKinds = 1, - properties = 2, - nodePropertyPrototypes = Array(Array.empty[String], Array.emptyShortArray), - edgePropertyPrototypes = Array(Array.empty[String]) - ) - val graph = new Graph(schema) +} + +object DiffToolTests { + val sampleSchema = TestSchema.make( + nodeKinds = 1, + edgeKinds = 1, + properties = 2, + nodePropertyPrototypes = Array(Array.empty[String], Array.emptyShortArray), + edgePropertyPrototypes = Array(Array.empty[String]) + ) + + def makeSampleGraph(): Graph = { + val graph = new Graph(sampleSchema) val v0 = new GenericDNode(0) val v1 = new GenericDNode(0) - DiffGraphApplier.applyDiff(graph, new DiffGraphBuilder(schema)._addEdge(v0, v1, 0, "edgePropertyValue")) + DiffGraphApplier.applyDiff(graph, new DiffGraphBuilder(sampleSchema)._addEdge(v0, v1, 0, "edgePropertyValue")) DiffGraphApplier.applyDiff( graph, - new DiffGraphBuilder(schema) + new DiffGraphBuilder(sampleSchema) ._setNodeProperty(v0.storedRef.get, 0, "A") ._setNodeProperty(v1.storedRef.get, 0, Seq("X", "Y")) ._setNodeProperty(v0.storedRef.get, 1, 40.toShort) ._setNodeProperty(v1.storedRef.get, 1, Seq(50.toShort, 51.toShort)) ) -// println(debugDump(graph)) + // println(debugDump(graph)) debugDump(graph) shouldBe """#Node numbers (kindId, nnodes) (0: 2), total 2 |Node kind 0. (eid, nEdgesOut, nEdgesIn): (0, 1 [dense], 1 [dense]), @@ -122,5 +127,4 @@ class DiffToolTests extends AnyWordSpec { testSerialization(graph) graph } - }