perf(core): optimize queryVerticesByIds and queryEdgesByIds for one id

[bitflicker64]

Purpose of the PR

Follow-up to #2859 (original work by @javeme), targeting its branch only-one-id-query-optimize so it can be merged into that PR. It replaces the implementation in that branch with a smaller one, fixes two semantic differences from master, and adds the tests and numbers that were asked for in the review. The author has been inactive on it since 2025-09 and it was relabeled help wanted. Note that #2859 itself still needs a rebase onto master (it conflicts with #2982 in queryEdgesByIds); the implementation here is written so that rebase is trivial, since the multi-id path and the distinctIds line are left untouched.

Querying a vertex or an edge by a single id is the hot path behind graph.vertex(id), graph.vertices(id), graph.edge(id), graph.adjacentVertex(id) and the lazy adjacent-vertex loading in HugeVertex.ensureFilledProperties(). It currently goes through the general multi-id code in GraphTransaction.queryVerticesByIds() / queryEdgesByIds() and allocates an IdQuery, an id list, a HashMap and a MapperIterator on every call.

Main Changes

• GraphTransaction: add a dedicated single-id path, queryVertexById() and queryEdgeById(), taken when exactly one id is passed. It does the three local-tx map lookups (only when the tx has changes) and, on a miss, a single IdQuery.OneIdQuery against the backend. The multi-id path is unchanged.
• The "vertex not found" handling (NotFoundException / undefined adjacent vertex / skip) is shared by both paths through resolveVertex().

Differences from the current #2859 code, on purpose:

1. Removed or expired records in the tx. perf(core): optimize queryVerticesByIds for only-one-id query #2859 put the id into the result id list before checking the removed/expired maps, so adjacentVertex(id) for a vertex deleted in the current tx returned HugeVertex.undefined(...) instead of nothing. With the default vertex.check_adjacent_vertex_exist=false that silently relabels the other vertex of a held edge to ~undefined and wipes its properties on the next property access. This PR keeps master's behaviour (nothing is returned), identical to the multi-id path. Covered by testQueryAdjacentVertexRemovedInLocalTx, testQuerySingleVertexByIdRemovedInLocalTx and the two ...ExpiredInLocalTx tests.
2. Single edge id from the backend. perf(core): optimize queryVerticesByIds for only-one-id query #2859 routed it through QueryResults.one(), which throws if the backend yields two results, while master returns the backend iterator as is. Every backend answers an exact edge id with at most one column (memory entry.contains(column), RocksDB getById, HStore getById) and queryEdgesFromBackendInternal already asserts that, so one() would most likely be safe, but there is no reason to change the contract in a perf PR: this PR returns the backend iterator directly, exactly like the multi-id path does for one id with no local hit. Note that this is guaranteed by construction (queryEdgeById() never calls one()), not by a test: no shipped backend can be made to yield two edges for one exact id, so a test would need a stubbed queryEdgesFromBackend(). For vertices the single-id backend path uses QueryResults.one(), which is what CachedGraphTransaction.queryVerticesByIds() already does today for every single-id query when the vertex cache is enabled (the default).
3. Not included: the 19 @Watched annotations, the prefix = "tx" to "graph" change on prepareCommit() (which would diverge from the 13 prefix = "tx" sites in AbstractTransaction), and the optimizeQuery() switch to OneIdQuery for primary-key lookups. They are unrelated to this optimization and can be separate PRs.
4. The two redundant asserts flagged by Copilot on perf(core): optimize queryVerticesByIds for only-one-id query #2859 are gone with the restructuring, and the boolean-flag control flow @imbajin commented on is replaced by early returns in the dedicated methods. I did not extract a shared findVertexInLocalTx() helper: the lookup has three outcomes (found, removed or expired, not tracked) so a helper would need a tri-state result or a second lookup, and folding the multi-id loops into it would touch code this PR otherwise leaves alone. If you would still like that, I am happy to do it here or as a follow-up.
5. The multi-id edge path keeps the distinctIds check from refactor(server): optimize rockdb batch query perf #2982: [a, b, a] still yields three edges (testQueryEdgesByNonConsecutiveDuplicateIds), and the same is now asserted for vertices and for mixed local/backend ids.

Benchmark

JMH, memory backend with default cache settings, 1000 vertices in a chain, single thread, 1 fork, 3 warmup + 5 measurement iterations of 1 s, Apple M-series, OpenJDK 11.0.31. clean = no uncommitted change in the tx, dirty = one uncommitted vertex and edge in the tx. Lower is better (ns/op).

Benchmark	tx	master (ns/op)	this PR (ns/op)	change
vertexByIdCommitted (graph.vertices(id), cached vertex)	clean	85.5 ± 0.9	29.0 ± 4.1	-66%
vertexByIdCommitted (graph.vertices(id), cached vertex)	dirty	83.5 ± 4.5	29.9 ± 8.6	-64%
vertexByIdStrict (graph.vertex(id))	clean	84.2 ± 5.7	22.5 ± 0.7	-73%
vertexByIdStrict (graph.vertex(id))	dirty	75.0 ± 0.2	25.7 ± 0.9	-66%
adjacentVertexById (graph.adjacentVertex(id))	clean	97.1 ± 4.8	22.3 ± 0.5	-77%
adjacentVertexById (graph.adjacentVertex(id))	dirty	80.7 ± 32.1	26.9 ± 0.3	-67%
vertexByIdMissing (graph.vertices(id), id not found)	clean	189.5 ± 8.4	116.8 ± 5.1	-38%
vertexByIdMissing (graph.vertices(id), id not found)	dirty	199.1 ± 25.3	110.6 ± 7.7	-44%
vertexByIdLocalTx (graph.vertices(id), id added in tx (dirty only))	dirty	47.3 ± 2.3	31.3 ± 37.5	-34%
edgeByIdCommitted (graph.edges(id), cached edge)	clean	491.8 ± 15.1	459.0 ± 142.5	-7%
edgeByIdCommitted (graph.edges(id), cached edge)	dirty	549.0 ± 39.2	449.3 ± 27.8	-18%
edgeByIdLocalTx (graph.edges(id), id added in tx (dirty only))	dirty	94.4 ± 7.4	19.2 ± 4.3	-80%
verticesByTwoIds (graph.vertices(id1, id2), multi-id path, unchanged)	clean	184.5 ± 8.6	176.9 ± 11.7	-4%
verticesByTwoIds (graph.vertices(id1, id2), multi-id path, unchanged)	dirty	189.5 ± 2.1	183.7 ± 20.4	-3%

The single-id vertex lookups drop from roughly 80 to 100 ns to 22 to 30 ns, mostly by not allocating an IdQuery, a list, a HashMap and a MapperIterator per call. A cached edge lookup is dominated by EdgeId parsing and the edge cache key, so it only gains 7 to 18 percent; the local-tx edge hit gains the most. The multi-id path is within noise, as expected.

The harness is QueryByIdBenchmark (not part of this PR, happy to add it under hugegraph-test/src/test/java/org/apache/hugegraph/benchmark if wanted).

Verifying these changes

• Need tests and can be verified as follows:
  • VertexCoreTest: testQuerySingleVertexByIdInLocalTx (added and updated), testQuerySingleVertexByIdRemovedInLocalTx, testQuerySingleVertexByIdNotFound, testQuerySingleVertexByNullId, testQuerySingleVertexByIdExpiredInLocalTx, testQueryVerticesByNonConsecutiveDuplicateIds, testQueryVerticesByIdsWithLocalAndDuplicateIds
  • EdgeCoreTest: testQuerySingleEdgeByIdInLocalTx, testQuerySingleEdgeByIdRemovedInLocalTx, testQuerySingleEdgeByIdNotFound, testQuerySingleEdgeByInvalidId, testQuerySingleEdgeByIdWithInDirection, testQuerySingleEdgeByIdExpiredInLocalTx, testQueryEdgesByIdsWithLocalAndDuplicateIds, testQueryAdjacentVertexRemovedInLocalTx
  • Ran locally on the memory backend (JDK 11) with the same change applied on top of current master: mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,memory, CoreTestSuite: 819 run, 0 failures, 0 errors, 94 skipped (the usual "Not support paging" / hstore-only skips). On this branch (same delta on the perf(core): optimize queryVerticesByIds for only-one-id query #2859 base): VertexCoreTest 268 run, 0 failures, 0 errors, 44 skipped. EdgeCoreTest 172 run, clean in 3 of 4 runs. One run had 2 errors, IllegalStateException: Graph ... has been closed, in the pre-existing testQueryEdgesByIdWithGraphAPI and testQueryEdgesByIdWithGraphAPIAndNotCommittedUpdate, both at their graph.edges(id, id) call, which is the multi-id path this PR does not touch. Both pass in isolation and in the three re-runs, and the unmodified branch head passed 2 of 2 runs (164 tests). I could not reproduce it again to get a full trace, so I am reporting it here rather than explaining it away. UnitTestSuite was not run in full; CachedGraphTransactionTest (the cache override of the single-id backend query): 13 run, 0 failures. Checkstyle (mvn validate) passes with 0 violations.
  • Not run: RocksDB, HBase and HStore backends. The change is backend-neutral (it only reorders which maps are consulted before the same backend query), so CI coverage on those is what I rely on.

Does this PR potentially affect the following parts?

• Nope

Documentation Status

• Doc - No Need

[bitflicker64]

Blocking: no. Summary: The two single-id fast paths match the multi-id semantics everywhere I could check them statically (removed/expired/local-tx handling, null ids, IN-direction switching, undefined adjacent vertices, queryVertex/queryEdge not-found), the @Watched set at this head is identical to master's, and OneIdQuery already carries mustSortByInput=false, so the backend contract is unchanged; two things need a second look — the restored multi-id edge path carries the pre-#2982 query.idsSize() check rather than master's distinctIds, which the description says otherwise, and all three build-server core-test jobs are red at this head. Evidence: read of GraphTransaction.java at 99c46af (:773-882 vertex paths, :988-1094 edge paths) against master GraphTransaction.java:769-827/933-992; IdQuery.java:92-107 and :129-190; MapperIterator.java:39-50; QueryResults.java:214-243; CachedGraphTransaction.java:317-387 for the cache override; #2982 patch for the distinctIds line; gh api repos/apache/hugegraph/contents/...?ref=only-one-id-query-optimize for the base-branch text; gh -R apache/hugegraph pr checks 3175 (build-server memory/rocksdb/hbase fail at "Run core test") vs gh -R apache/hugegraph pr checks 2859 on the same base (all three pass). The failing job logs could not be read in this session, so the cause of the CI failure is not established here.

[bitflicker64]

⚠️ This is the pre-#2982 duplicate-id check, not the distinctIds one the description says it is.

Master carries Set<Id> distinctIds = InsertionOrderUtil.newSet(edgeIds.length) (GraphTransaction.java:940), distinctIds.add(id) (:967) and if (edges.isEmpty() && distinctIds.size() == ids.size()) (:972). #2982 changed exactly this line, from query.idsSize() == ids.size() to the distinctIds form. This head has the old form.

Why the two are not equivalent: IdQuery.query(Id) skips a duplicate only when it is consecutive (IdQuery.java:98-102). For graph.edges(a, b, a) with nothing in the local tx, the query ends up holding [a, b, a], so query.idsSize() is 3, ids.size() is 3, and this branch hands the backend iterator straight back for a query that contains a duplicate id — the batched-lookup case #2982 fixed.

Nothing regresses against this PR's base: only-one-id-query-optimize predates #2982 and already reads query.idsSize() == ids.size() at its GraphTransaction.java:1115, and the diff only re-indents the line. The problem is the description, which says "The multi-id edge path keeps the distinctIds check from #2982: [a, b, a] still yields three edges (testQueryEdgesByNonConsecutiveDuplicateIds)". Neither distinctIds nor that test exists at this head or on the base branch — only the vertex counterpart was added (VertexCoreTest.java:3131 testQueryVerticesByNonConsecutiveDuplicateIds). So the rebase onto master that #2859 still needs has nothing to catch it if this side of the conflict wins.

Requested change: either bring master's distinctIds set onto this line now and add the edge counterpart of testQueryVerticesByNonConsecutiveDuplicateIds, or correct that bullet in the PR description so whoever rebases #2859 knows this specific line has to come from master rather than from this branch.

[bitflicker64]

🧹 The fast path resolves eagerly, so NotFoundException now fires at call time instead of on iteration.

queryVerticesByIds previously returned a lazy MapperIterator in every case, and the checkMustExist throw happened inside the mapper — that is, on the first hasNext()/next() (MapperIterator.fetch(), hugegraph-commons/.../iterator/MapperIterator.java:39-50). Here resolveVertex runs before the iterator is constructed, so queryVerticesByIds(...) itself throws.

queryVertex(Object) is unaffected: it consumes with QueryResults.one(iter) on the next line (GraphTransaction.java:734-742). The visible difference is through queryAdjacentVertices(Object...) (:725-728), which passes checkMustExist = this.checkAdjacentVertexExist — with vertex.check_adjacent_vertex_exist=true (not the default), graph.adjacentVertex(missingId) raises when the iterator is built rather than when it is read. It also leaves the one-id and many-id paths throwing at different moments for the same call, which is the kind of difference the rest of this change is careful to avoid.

Requested change: keep the resolution inside the returned iterator — a small lazy single-element iterator preserves the allocation win this method is after — or, if the eager throw is intentional, say so in the javadoc above so it reads as a deliberate contract change rather than a side effect of the optimization.

[imbajin]

Blocking: no. Summary: The dedicated single-ID path is covered by tests, but the multi-ID path now performs transaction-map lookups even when the transaction is clean, adding work to the path that was intended to remain unchanged. Evidence: git diff HEAD^ HEAD at GraphTransaction.java:796 and :1011 removes the previous verticesInTxSize()/edgesInTxSize() > 0 guards; the clean path previously skipped these maps.

[imbajin]

⚠️ This removes the verticesUpdated guard from the multi-ID path. On every clean transaction, graph.vertices(id1, id2, ...) now calls removedVertices.containsKey(id) and then up to two local-map gets for each ID, whereas the previous code skipped all three maps when verticesInTxSize() == 0. The PR describes the multi-ID path as unchanged, and this adds overhead to batched lookups. Please retain the size guard around local-TX checks while keeping the new single-ID fast path.

[imbajin]

⚠️ This removes the edgesUpdated guard from the multi-ID path. On every clean transaction, graph.edges(id1, id2, ...) now calls removedEdges.containsKey(id) and then up to two local-map gets for each ID, whereas the previous code skipped all three maps when edgesInTxSize() == 0. The PR describes the multi-ID path as unchanged, and this adds overhead to batched lookups. Please retain the size guard around local-TX checks while keeping the new single-ID fast path.