From e81b648f5f8bdcada7fd4e5f0fcb85f975024aa7 Mon Sep 17 00:00:00 2001 From: ioannispan Date: Wed, 11 Oct 2023 08:59:49 +0200 Subject: [PATCH] Exhibit issue and propose fix --- .../core/huge/CompositeAdjacencyCursor.java | 5 ++-- .../gds/core/huge/CompositeAdjacencyList.java | 10 ++++---- .../core/huge/CompositeAdjacencyListTest.java | 25 ++++++++++++++++++- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java index 427b80dfae5..b55d835f9cc 100644 --- a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java +++ b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyCursor.java @@ -51,9 +51,10 @@ public List cursors() { return cursors; } - void updateCursorsQueue() { + void reinitializeCursors() { cursorQueue.clear(); - cursorQueue.addAll(cursors); + initializeQueue(); + } @Override public int size() { diff --git a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java index 48edf67b092..2d5d22514d6 100644 --- a/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java +++ b/core/src/main/java/org/neo4j/gds/core/huge/CompositeAdjacencyList.java @@ -126,19 +126,19 @@ public CompositeAdjacencyCursor adjacencyCursor(@Nullable AdjacencyCursor reuse, if (reuse instanceof CompositeAdjacencyCursor) { var compositeReuse = (CompositeAdjacencyCursor) reuse; var iter = compositeReuse.cursors().listIterator(); - boolean cursorsAreUpdated = false; + while (iter.hasNext()) { var index = iter.nextIndex(); var cursor = iter.next(); var newCursor = adjacencyLists.get(index).adjacencyCursor(cursor, node, fallbackValue); if (newCursor != cursor) { iter.set(adjacencyCursorWrapperFactory.create(newCursor)); - cursorsAreUpdated = true; } } - if (cursorsAreUpdated) { - compositeReuse.updateCursorsQueue(); - } + //we must update all the cursors all the time because old cursor might be further ahead instead of start + //then, object equality can be then the same, but in reality not so much + compositeReuse.reinitializeCursors(); + return compositeReuse; } return adjacencyCursor(node, fallbackValue); diff --git a/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java b/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java index 2b2751cbcf5..cd3abfed391 100644 --- a/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java +++ b/core/src/test/java/org/neo4j/gds/core/huge/CompositeAdjacencyListTest.java @@ -20,11 +20,12 @@ package org.neo4j.gds.core.huge; import org.junit.jupiter.api.Test; +import org.neo4j.gds.api.Graph; import org.neo4j.gds.extension.GdlExtension; import org.neo4j.gds.extension.GdlGraph; import org.neo4j.gds.extension.Inject; -import org.neo4j.gds.api.Graph; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @GdlExtension @@ -47,4 +48,26 @@ void shouldIgnoreInputDegreeForCursor() { var cursor = adjacencyList.adjacencyCursor(0); assertEquals(2, cursor.remaining()); } + + @Test + void shouldReuseCursor() { + + var adjacencyList = ((UnionGraph) graph).relationshipTopology(); + var cursor = adjacencyList.adjacencyCursor(null, 0); + assertThat(cursor.peekVLong()).isEqualTo(1L); + cursor.nextVLong(); + + cursor = adjacencyList.adjacencyCursor(cursor, 0); + assertThat(cursor.peekVLong()).isEqualTo(1L); + + + cursor.nextVLong(); + cursor.nextVLong(); + + cursor = adjacencyList.adjacencyCursor(cursor, 0); + assertThat(cursor.hasNextVLong()).isTrue(); + + + } + }