From ceb0a100e83fffaac66de606337ea5aeaed791ad Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 29 Aug 2026 20:36:12 +0530 Subject: [PATCH] perf(core): optimize queryVerticesByIds and queryEdgesByIds for one id Querying a vertex or an edge by a single id is the hot path behind graph.vertex(id), graph.vertices(id), graph.edge(id) and the adjacent vertex loading in HugeVertex.ensureFilledProperties(). It went through the general multi-id code and allocated an id list, a HashMap and a MapperIterator on every call. Add a dedicated single-id path for vertices and edges that keeps the semantics of the multi-id path (removed, updated, expired and missing records, null and invalid ids, IN direction edge ids, undefined adjacent vertices, NotFoundException when the record must exist) but only does the map lookups and, on a miss, one OneIdQuery against the backend. The multi-id path is unchanged. The "vertex not found" handling (NotFoundException, undefined adjacent vertex, or skip) is shared by both paths. Supersedes #2859, original work by Jermy Li (javeme). Co-authored-by: Jermy Li --- .../backend/tx/GraphTransaction.java | 126 ++++++++-- .../apache/hugegraph/core/EdgeCoreTest.java | 216 ++++++++++++++++++ .../apache/hugegraph/core/VertexCoreTest.java | 171 ++++++++++++++ 3 files changed, 500 insertions(+), 13 deletions(-) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index 0c962b11a2..ff11685636 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -775,6 +775,12 @@ protected Iterator queryVerticesByIds(Object[] vertexIds, boolean adjace boolean checkMustExist, HugeType type) { Query.checkForceCapacity(vertexIds.length); + if (vertexIds.length == 1) { + // Fast path: skip the id list, map and mapper iterator for one id + return this.queryVertexById(vertexIds[0], adjacentVertex, + checkMustExist, type); + } + // NOTE: allowed duplicated vertices if query by duplicated ids List ids = InsertionOrderUtil.newList(); Map vertices = new HashMap<>(vertexIds.length); @@ -808,22 +814,67 @@ protected Iterator queryVerticesByIds(Object[] vertexIds, boolean adjace } return new MapperIterator<>(ids.iterator(), id -> { - HugeVertex vertex = vertices.get(id); + return this.resolveVertex(vertices.get(id), id, + adjacentVertex, checkMustExist); + }); + } + + /** + * Query a single vertex by id, with the same semantics as the multi-id + * path of {@link #queryVerticesByIds(Object[], boolean, boolean, HugeType)} + * but without allocating the id list, the result map and the mapper + * iterator; only an {@link IdQuery.OneIdQuery} is created on a miss. + */ + private Iterator queryVertexById(Object vertexId, boolean adjacentVertex, + boolean checkMustExist, HugeType type) { + Id id = HugeVertex.getIdValue(vertexId); + if (id == null) { + return QueryResults.emptyIterator(); + } + + HugeVertex vertex = null; + if (this.verticesInTxSize() > 0) { + if (this.removedVertices.containsKey(id)) { + // The record has been deleted + return QueryResults.emptyIterator(); + } + vertex = this.addedVertices.get(id); if (vertex == null) { - if (checkMustExist) { - throw new NotFoundException( - "Vertex '%s' does not exist", id); - } else if (adjacentVertex) { - assert !checkMustExist; - // Return undefined if adjacentVertex but !checkMustExist - vertex = HugeVertex.undefined(this.graph(), id); - } else { - // Return null - assert vertex == null; - } + vertex = this.updatedVertices.get(id); } + if (vertex != null && vertex.expired()) { + // Found from local tx but expired + return QueryResults.emptyIterator(); + } + } + + if (vertex == null) { + // Query from backend store + IdQuery query = new IdQuery.OneIdQuery(type, id); + vertex = QueryResults.one(this.queryVerticesFromBackend(query)); + } + + vertex = this.resolveVertex(vertex, id, adjacentVertex, checkMustExist); + if (vertex == null) { + return QueryResults.emptyIterator(); + } + return QueryResults.iterator(vertex); + } + + private HugeVertex resolveVertex(HugeVertex vertex, Id id, + boolean adjacentVertex, boolean checkMustExist) { + if (vertex != null) { return vertex; - }); + } + if (checkMustExist) { + throw new NotFoundException("Vertex '%s' does not exist", id); + } + if (adjacentVertex) { + // Return undefined if adjacentVertex but !checkMustExist + return HugeVertex.undefined(this.graph(), id); + } + // Return null to skip the vertex + return null; } public Iterator queryVertices() { @@ -934,6 +985,11 @@ protected Iterator queryEdgesByIds(Object[] edgeIds, boolean verifyId) { Query.checkForceCapacity(edgeIds.length); + if (edgeIds.length == 1) { + // Fast path: skip the id list, map and mapper iterator for one id + return this.queryEdgeById(edgeIds[0], verifyId); + } + // NOTE: allowed duplicated edges if query by duplicated ids List ids = InsertionOrderUtil.newList(); Map edges = new HashMap<>(edgeIds.length); @@ -991,6 +1047,50 @@ protected Iterator queryEdgesByIds(Object[] edgeIds, }); } + /** + * Query a single edge by id, with the same semantics as the multi-id + * path of {@link #queryEdgesByIds(Object[], boolean)} but without + * allocating the id list, the result map and the mapper iterator; only + * an {@link IdQuery.OneIdQuery} is created on a miss. + */ + private Iterator queryEdgeById(Object edgeId, boolean verifyId) { + EdgeId id = HugeEdge.getIdValue(edgeId, !verifyId); + if (id == null) { + return QueryResults.emptyIterator(); + } + if (id.direction() == Directions.IN) { + id = id.switchDirection(); + } + + if (this.edgesInTxSize() > 0) { + if (this.removedEdges.containsKey(id)) { + // The record has been deleted + return QueryResults.emptyIterator(); + } + HugeEdge edge = this.addedEdges.get(id); + if (edge == null) { + edge = this.updatedEdges.get(id); + } + if (edge != null) { + // Found from local tx + if (edge.expired()) { + return QueryResults.emptyIterator(); + } + return QueryResults.iterator(edge); + } + } + + /* + * Query from backend store and return the results directly, just + * like the multi-id path does when there is no local edge. + */ + IdQuery query = new IdQuery.OneIdQuery(HugeType.EDGE, id); + Iterator it = this.queryEdgesFromBackend(query); + @SuppressWarnings({"unchecked", "rawtypes"}) + Iterator r = (Iterator) it; + return r; + } + public Iterator queryEdges() { Query q = new Query(HugeType.EDGE); return this.queryEdges(q); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java index cbb2b7d043..238e5a6eb0 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; @@ -33,6 +34,7 @@ import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.backend.BackendException; +import org.apache.hugegraph.backend.id.EdgeId; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; import org.apache.hugegraph.backend.page.PageInfo; @@ -46,6 +48,7 @@ import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.exception.LimitExceedException; import org.apache.hugegraph.exception.NoIndexException; +import org.apache.hugegraph.exception.NotFoundException; import org.apache.hugegraph.schema.SchemaManager; import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.structure.HugeEdge; @@ -2727,6 +2730,176 @@ public void testQueryEdgesByIdWithGraphAPIAndNotCommittedRemoved() { Assert.assertTrue(graph.edges(edge1.id(), edge2.id()).hasNext()); } + @Test + public void testQueryEdgesByIdsWithLocalAndDuplicateIds() { + HugeGraph graph = graph(); + init18Edges(); + + Object id1 = graph.traversal().E().toList().get(0).id(); + // Added but not committed + Vertex james = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Vertex book = graph.addVertex(T.label, "book", "name", "c-book"); + Edge local = james.addEdge("authored", book, "score", 5); + Object id2 = local.id(); + + List edges = ImmutableList.copyOf(graph.edges(id2, id1, id2)); + Assert.assertEquals(3, edges.size()); + Assert.assertSame(local, edges.get(0)); + Assert.assertEquals(id1, edges.get(1).id()); + Assert.assertSame(local, edges.get(2)); + + graph.tx().rollback(); + } + + @Test + public void testQuerySingleEdgeByIdInLocalTx() { + HugeGraph graph = graph(); + init18Edges(); + + // Added but not committed + Vertex james = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Vertex book = graph.addVertex(T.label, "book", "name", "c-book"); + Edge edge = james.addEdge("authored", book, "score", 5); + Object id = edge.id(); + + List edges = ImmutableList.copyOf(graph.edges(id)); + Assert.assertEquals(1, edges.size()); + Assert.assertSame(edge, edges.get(0)); + Assert.assertSame(edge, graph.edge(id)); + graph.tx().commit(); + + // Updated but not committed + edge = graph.edge(id); + edge.property("score", 6); + + edges = ImmutableList.copyOf(graph.edges(id)); + Assert.assertEquals(1, edges.size()); + Assert.assertSame(edge, edges.get(0)); + Assert.assertEquals(6, (int) graph.edge(id).value("score")); + graph.tx().rollback(); + + Assert.assertEquals(5, (int) graph.edge(id).value("score")); + } + + @Test + public void testQuerySingleEdgeByIdRemovedInLocalTx() { + HugeGraph graph = graph(); + init18Edges(); + + Edge edge = graph.traversal().E().toList().get(0); + Object id = edge.id(); + Assert.assertTrue(graph.edges(id).hasNext()); + + edge.remove(); + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + graph.tx().rollback(); + Assert.assertTrue(graph.edges(id).hasNext()); + Assert.assertEquals(id, graph.edge(id).id()); + } + + @Test + public void testQuerySingleEdgeByIdNotFound() { + HugeGraph graph = graph(); + init18Edges(); + + String id = graph.traversal().E().toList().get(0).id() + "-not-exist"; + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + } + + @Test + public void testQuerySingleEdgeByInvalidId() { + HugeGraph graph = graph(); + init18Edges(); + + // Invalid id is skipped by edges() and rejected by edge() + Assert.assertFalse(graph.edges("invalid-edge-id").hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge("invalid-edge-id"); + }, e -> { + Assert.assertContains("Edge id must be formatted", e.getMessage()); + }); + + Assert.assertFalse(graph.edges((Object) null).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(null); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + } + + @Test + public void testQuerySingleEdgeByIdWithInDirection() { + HugeGraph graph = graph(); + init18Edges(); + + HugeEdge edge = (HugeEdge) graph.traversal().E().toList().get(0); + EdgeId inId = edge.idWithDirection().switchDirection(); + Assert.assertEquals(Directions.IN, inId.direction()); + + List edges = ImmutableList.copyOf(graph.edges(inId)); + Assert.assertEquals(1, edges.size()); + Assert.assertEquals(edge.id(), edges.get(0).id()); + Assert.assertEquals(edge.id(), graph.edge(inId).id()); + + edges = ImmutableList.copyOf(graph.edges(inId.asString())); + Assert.assertEquals(1, edges.size()); + Assert.assertEquals(edge.id(), edges.get(0).id()); + } + + @Test + public void testQuerySingleEdgeByIdExpiredInLocalTx() { + HugeGraph graph = graph(); + + Vertex baby = graph.addVertex(T.label, "person", "name", "Baby", + "age", 3, "city", "Beijing"); + Vertex java = graph.addVertex(T.label, "book", + "name", "Java in action"); + Edge edge = baby.addEdge("read", java, "place", "library of school", + "date", "2019-12-23 12:00:00"); + Object id = edge.id(); + graph.tx().commit(); + + edge = graph.edge(id); + Assert.assertTrue(graph.edges(id).hasNext()); + graph.tx().rollback(); + + try { + Thread.sleep(3100L); + } catch (InterruptedException e) { + // Ignore + } + + // Update the expired edge in a new tx (not committed) + edge.property("place", "home"); + Map updated = Whitebox.getInternalState( + params().graphTransaction(), "updatedEdges"); + Assert.assertTrue(updated.containsKey(edge.id())); + Assert.assertTrue(((HugeEdge) edge).expired()); + + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + graph.tx().rollback(); + } + @Test public void testQueryEdgesByIdNotFound() { HugeGraph graph = graph(); @@ -3322,6 +3495,49 @@ public void testQueryAdjacentVerticesOfEdgesWithoutVertex() } } + @Test + public void testQueryAdjacentVertexRemovedInLocalTx() + throws InterruptedException, ExecutionException { + HugeGraph graph = graph(); + + Vertex james = graph.addVertex(T.label, "author", "id", 1, + "name", "James Gosling", "age", 62, + "lived", "Canadian"); + Vertex java = graph.addVertex(T.label, "book", "name", "java"); + james.addEdge("authored", java, "score", 3); + graph.tx().commit(); + params().graphEventHub().notify(Events.CACHE, "clear", null).get(); + + Edge edge = graph.traversal().V(james.id()).outE().next(); + HugeVertex adjacent = (HugeVertex) edge.inVertex(); + Assert.assertFalse(adjacent.isPropLoaded()); + Assert.assertEquals("book", adjacent.label()); + + // Remove the adjacent vertex but don't commit + graph.vertex(java.id()).remove(); + Assert.assertFalse(graph.vertices(java.id()).hasNext()); + Assert.assertFalse(graph.adjacentVertex(java.id()).hasNext()); + // Querying by one id must agree with querying by multiple ids + List vertices = ImmutableList.copyOf( + params().graphTransaction() + .queryAdjacentVertices(java.id(), james.id())); + Assert.assertEquals(1, vertices.size()); + Assert.assertEquals(james.id(), vertices.get(0).id()); + + /* + * Loading the adjacent vertex of an edge held before the removal + * must not turn it into an undefined vertex + */ + adjacent.forceLoad(); + Assert.assertFalse(adjacent.schemaLabel().undefined()); + Assert.assertEquals("book", adjacent.label()); + + graph.tx().rollback(); + Assert.assertTrue(graph.vertices(java.id()).hasNext()); + Assert.assertEquals("book", graph.adjacentVertex(java.id()) + .next().label()); + } + @Test public void testQueryAdjacentVerticesOfEdgesWithInvalidVertexLabel() throws InterruptedException, ExecutionException { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java index 9aa144542e..6feceee51b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Random; @@ -52,11 +53,13 @@ import org.apache.hugegraph.exception.LimitExceedException; import org.apache.hugegraph.exception.NoIndexException; import org.apache.hugegraph.exception.NotAllowException; +import org.apache.hugegraph.exception.NotFoundException; import org.apache.hugegraph.schema.PropertyKey; import org.apache.hugegraph.schema.SchemaManager; import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.schema.VertexLabel; import org.apache.hugegraph.structure.HugeElement; +import org.apache.hugegraph.structure.HugeVertex; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.FakeObjects; import org.apache.hugegraph.testutil.Utils; @@ -3188,6 +3191,174 @@ public void testQueryByIdWithGraphAPIAndNotCommittedRemoved() { Assert.assertTrue(graph.vertices(vertex1.id(), vertex2.id()).hasNext()); } + @Test + public void testQueryVerticesByNonConsecutiveDuplicateIds() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + List all = graph.traversal().V().toList(); + Object id1 = all.get(0).id(); + Object id2 = all.get(1).id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id1, id2, id1)); + Assert.assertEquals(3, vertices.size()); + Assert.assertEquals(id1, vertices.get(0).id()); + Assert.assertEquals(id2, vertices.get(1).id()); + Assert.assertEquals(id1, vertices.get(2).id()); + } + + @Test + public void testQueryVerticesByIdsWithLocalAndDuplicateIds() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Object id1 = graph.traversal().V().toList().get(0).id(); + // Added but not committed + Vertex local = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Object id2 = local.id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id2, id1, id2)); + Assert.assertEquals(3, vertices.size()); + Assert.assertSame(local, vertices.get(0)); + Assert.assertEquals(id1, vertices.get(1).id()); + Assert.assertSame(local, vertices.get(2)); + + graph.tx().rollback(); + } + + @Test + public void testQuerySingleVertexByIdInLocalTx() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + // Added but not committed + Vertex vertex = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Object id = vertex.id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertSame(vertex, vertices.get(0)); + Assert.assertSame(vertex, graph.vertex(id)); + Assert.assertSame(vertex, graph.adjacentVertex(id).next()); + this.commitTx(); + + // Updated but not committed + vertex = graph.vertex(id); + vertex.property("age", 71); + + vertices = ImmutableList.copyOf(graph.vertices(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertSame(vertex, vertices.get(0)); + Assert.assertEquals(71, (int) graph.vertex(id).value("age")); + graph.tx().rollback(); + + Assert.assertEquals(70, (int) graph.vertex(id).value("age")); + } + + @Test + public void testQuerySingleVertexByIdRemovedInLocalTx() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Vertex vertex = graph.traversal().V().hasLabel("author") + .has("id", 1).next(); + Object id = vertex.id(); + Assert.assertTrue(graph.vertices(id).hasNext()); + Assert.assertTrue(graph.adjacentVertex(id).hasNext()); + + vertex.remove(); + Assert.assertFalse(graph.vertices(id).hasNext()); + // Removed vertex must not be reported as an undefined adjacent vertex + Assert.assertFalse(graph.adjacentVertex(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + graph.tx().rollback(); + Assert.assertTrue(graph.vertices(id).hasNext()); + Assert.assertTrue(graph.adjacentVertex(id).hasNext()); + } + + @Test + public void testQuerySingleVertexByIdNotFound() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Id id = SplicingIdGenerator.splicing("author", "not-exists-id"); + Assert.assertFalse(graph.vertices(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + // Adjacent vertex not found is returned as an undefined vertex + // (vertex.check_adjacent_vertex_exist=false, the default) + List vertices = ImmutableList.copyOf(graph.adjacentVertex(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertEquals(id, vertices.get(0).id()); + Assert.assertEquals("~undefined", vertices.get(0).label()); + } + + @Test + public void testQuerySingleVertexByNullId() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Assert.assertFalse(graph.vertices((Object) null).hasNext()); + Assert.assertFalse(graph.adjacentVertex(null).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(null); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + // Same as querying by multiple ids + Assert.assertFalse(graph.vertices(null, null).hasNext()); + } + + @Test + public void testQuerySingleVertexByIdExpiredInLocalTx() { + HugeGraph graph = graph(); + + Vertex vertex = graph.addVertex(T.label, "fan", "name", "Baby", + "age", 3, "city", "Beijing"); + Object id = vertex.id(); + this.commitTx(); + + vertex = graph.vertex(id); + Assert.assertTrue(graph.vertices(id).hasNext()); + graph.tx().rollback(); + + try { + Thread.sleep(3100L); + } catch (InterruptedException e) { + // Ignore + } + + // Update the expired vertex in a new tx (not committed) + vertex.property("age", 4); + Map updated = Whitebox.getInternalState( + params().graphTransaction(), "updatedVertices"); + Assert.assertTrue(updated.containsKey(vertex.id())); + Assert.assertTrue(((HugeVertex) vertex).expired()); + + Assert.assertFalse(graph.vertices(id).hasNext()); + Assert.assertFalse(graph.adjacentVertex(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + graph.tx().rollback(); + } + @Test public void testQueryByInvalidSysprop() { HugeGraph graph = graph();