perf(core): optimize queryVerticesByIds for only-one-id query

[javeme]

Change-Id: Ic6e01eb8e2b4c97c5263cfa0f78c58b6033435da

[imbajin]

Consider extracting the local transaction lookup logic to reduce code duplication between single-ID and multi-ID paths. This pattern repeats in both vertex and edge queries:

private HugeVertex findVertexInLocalTx(Id id) { if (this.removedVertices.containsKey(id)) { return null; // Removed } HugeVertex vertex = this.addedVertices.get(id); if (vertex == null) { vertex = this.updatedVertices.get(id); } return (vertex != null && !vertex.expired()) ? vertex : null; }

This would simplify the control flow and make the code more maintainable.

[imbajin]

The multiple boolean flags (tryQueryBackend, foundLocal) make the control flow complex. Consider refactoring into a more readable structure:

private QueryResult<HugeVertex> querySingleVertex(Id id, HugeType type) { if (id == null) { return QueryResult.empty(); } HugeVertex localVertex = findVertexInLocalTx(id); if (localVertex != null) { return QueryResult.fromLocal(localVertex); } if (isRemovedFromTx(id)) { return QueryResult.empty(); } return QueryResult.fromBackend(queryBackendVertex(id, type)); }

This would eliminate the complex flag management and make the logic clearer.

[imbajin]

Missing test coverage for the single-ID optimization. Consider adding tests for:

1. Performance verification: Ensure single-ID queries are faster than multi-ID queries
2. Correctness tests:
   • Single ID found in local transaction (added/updated)
   • Single ID removed from local transaction
   • Single ID not found anywhere
   • Single null ID handling
3. Edge cases: Single ID with expired vertex

Example test structure:

@Test public void testSingleVertexQueryOptimization() { // Test single vertex query performance and correctness Id vertexId = IdGenerator.of("test-vertex"); // Test local transaction lookup HugeVertex localVertex = constructVertex(vertexId); tx.addVertex(localVertex); Iterator<Vertex> result = tx.queryVertices(vertexId); assertThat(result).hasNext(); assertThat(result.next()).isEqualTo(localVertex); }

[imbajin]

Consider optimizing the verticesUpdated check. Currently it calls this.verticesInTxSize() > 0 which may iterate through all transaction maps. For single-ID queries, you could check more efficiently:

// More efficient check for single ID boolean hasLocalChanges = this.removedVertices.containsKey(id) || this.addedVertices.containsKey(id) || this.updatedVertices.containsKey(id);

This would be O(1) instead of potentially O(n) for the transaction size calculation.

[imbajin]

The same optimization pattern is duplicated for edges and vertices. Consider creating a generic helper method to reduce code duplication:

private <T extends HugeElement> Map<Id, T> querySingleElement( Id id, HugeType type, Map<Id, T> added, Map<Id, T> updated, Map<Id, T> removed, Function<IdQuery, Iterator<T>> backendQuery) { if (id == null) { return ImmutableMap.of(); } // Check local transaction state if (removed.containsKey(id)) { return ImmutableMap.of(); } T element = added.get(id); if (element == null) { element = updated.get(id); } if (element != null && !element.expired()) { return ImmutableMap.of(id, element); } // Query backend IdQuery query = new IdQuery.OneIdQuery(type, id); T backendElement = QueryResults.one(backendQuery.apply(query)); return backendElement == null ? ImmutableMap.of() : ImmutableMap.of(id, backendElement); }

This would eliminate the duplication between vertex and edge query methods.

[Copilot]

Pull Request Overview

This PR optimizes the queryVerticesByIds and queryEdgesByIds methods to handle single-ID queries more efficiently by introducing special handling for cases where only one ID is being queried.

• Optimizes single-ID queries by avoiding unnecessary data structures and using ImmutableMap for single results
• Refactors the existing multi-ID query logic into conditional branches
• Updates monitoring annotations from "tx" to "graph" prefix and adds new @Watched annotations

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

This assertion is redundant since the condition vertex.expired() being false already guarantees that vertex is not null at this point.

Suggested change

assert vertex != null;

[Copilot]

This assertion is redundant since the condition edge.expired() being false already guarantees that edge is not null at this point.

Suggested change

	} else {
	assert edge != null;

[github-actions]

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label