Performance
This commit is contained in:
@@ -4,4 +4,4 @@
|
||||
server.url=http://localhost:8787
|
||||
# Stamped by manage-ac.sh (stamp_cli_version) from ac-code-server's agenticcode.version
|
||||
# at build time. "dev" means this jar wasn't built via manage-ac.sh.
|
||||
version=157
|
||||
version=162
|
||||
|
||||
@@ -3,7 +3,7 @@ quarkus.http.port=8787
|
||||
# AgenticCode's own release counter (not the Maven project version) — bump this by hand for each
|
||||
# release. Single source of truth for the startup log line, GET /api/version, and the OpenAPI
|
||||
# info version (referenced below via property expression, not duplicated).
|
||||
agenticcode.version=157
|
||||
agenticcode.version=162
|
||||
# OpenAPI / Swagger UI (item 48) — the generated spec is the contract the web-UI TS client
|
||||
# is generated against. Served at /q/openapi (yaml/json); Swagger UI at /q/swagger-ui in dev.
|
||||
mp.openapi.extensions.smallrye.info.title=AgenticCode API
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
package com.agenticcode.codeserver.api;
|
||||
|
||||
import com.agenticcode.neo4jstore.graph.GraphRepository;
|
||||
import io.quarkus.test.junit.QuarkusTest;
|
||||
import io.restassured.RestAssured;
|
||||
import jakarta.inject.Inject;
|
||||
import org.junit.jupiter.api.BeforeAll;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
import org.neo4j.driver.Driver;
|
||||
import org.neo4j.driver.Session;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.UncheckedIOException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import static io.restassured.RestAssured.given;
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Stage 1 of the {@code type}-as-label migration: every persisted node carries its
|
||||
* {@link com.agenticcode.parsercore.ast.model.NodeType} as a Neo4j label, in addition to the
|
||||
* unchanged {@code type} property.
|
||||
*
|
||||
* <p>The point is to stop paying a property-store read for what a label answers for free. Profiled
|
||||
* on {@code upms}, a single {@code CALLS} traversal spent ~935k of its 1.6M dbHits (58%) reading the
|
||||
* {@code type} property off candidate nodes, because it sits in an 11-property chain.
|
||||
*
|
||||
* <p>Stage 1 is deliberately additive: the property stays, so none of the ~200 existing queries
|
||||
* change behaviour. This test guards that both halves hold — the label is there, and the property
|
||||
* still agrees with it.
|
||||
*/
|
||||
@QuarkusTest
|
||||
class TypeLabelIT {
|
||||
|
||||
private static final String PROJECT = "type-label-stage1";
|
||||
|
||||
@TempDir
|
||||
static Path root;
|
||||
|
||||
@Inject
|
||||
GraphRepository graphRepository;
|
||||
|
||||
@Inject
|
||||
Driver driver;
|
||||
|
||||
@BeforeAll
|
||||
static void createProject() {
|
||||
RestAssured.port = Integer.getInteger("quarkus.http.test-port", 8081);
|
||||
// Covers both persist paths: MERGE_NODES for MODULE/FUNCTION/VARIABLE, and
|
||||
// MERGE_POSITIONAL_NODES for the positional types — DB_ACCESS via the FIND, and CONTROL_FLOW
|
||||
// via the IF, both of which are keyed by startLine rather than name alone.
|
||||
write("TL_MAIN.nat", """
|
||||
DEFINE DATA
|
||||
LOCAL
|
||||
1 #CLIENT (A8)
|
||||
1 EMPLOYEES VIEW OF EMPLOYEES
|
||||
2 PERSONNEL-ID
|
||||
END-DEFINE
|
||||
*
|
||||
FIND EMPLOYEES WITH PERSONNEL-ID = #CLIENT
|
||||
IF #CLIENT = 'X'
|
||||
IGNORE
|
||||
END-IF
|
||||
END-FIND
|
||||
*
|
||||
DEFINE SUBROUTINE TL-SUB
|
||||
IGNORE
|
||||
END-SUBROUTINE
|
||||
*
|
||||
END
|
||||
""");
|
||||
given().contentType("application/json")
|
||||
.body(new ProjectResource.ProjectRequest(null, root.toString(), null, "natural", null, null))
|
||||
.when().post("/api/projects/" + PROJECT)
|
||||
.then().statusCode(201);
|
||||
// Coarse scan alone does not produce the positional types; a deep refresh does.
|
||||
given().when().post("/api/projects/" + PROJECT + "/refresh?deep=true")
|
||||
.then().statusCode(200);
|
||||
}
|
||||
|
||||
private static void write(String fileName, String content) {
|
||||
try {
|
||||
Files.write(root.resolve(fileName), content.getBytes(StandardCharsets.UTF_8));
|
||||
} catch (IOException e) {
|
||||
throw new UncheckedIOException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void everyNodeCarriesItsTypeAsLabel() {
|
||||
try (Session session = driver.session()) {
|
||||
long mismatched = session.run(
|
||||
"MATCH (n:AstNode {project: $p}) WHERE NOT n.type IN labels(n) RETURN count(n) AS c",
|
||||
Map.of("p", PROJECT)).single().get("c").asLong();
|
||||
assertEquals(0, mismatched, "every persisted node must carry its type as a label");
|
||||
|
||||
long total = session.run("MATCH (n:AstNode {project: $p}) RETURN count(n) AS c",
|
||||
Map.of("p", PROJECT)).single().get("c").asLong();
|
||||
assertTrue(total > 0, "fixture must actually have produced nodes");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Both persist paths, not just the common one: {@code MERGE_POSITIONAL_NODES} is a separate
|
||||
* statement and would silently miss its {@code SET node:$(n.type)} without this.
|
||||
*/
|
||||
@Test
|
||||
void bothPersistPathsLabelTheirNodes() {
|
||||
try (Session session = driver.session()) {
|
||||
for (String type : List.of("MODULE", "FUNCTION", "VARIABLE", "DB_ACCESS", "CONTROL_FLOW")) {
|
||||
long labelled = session.run(
|
||||
"MATCH (n:AstNode {project: $p, type: $t}) WHERE $t IN labels(n) RETURN count(n) AS c",
|
||||
Map.of("p", PROJECT, "t", type)).single().get("c").asLong();
|
||||
assertTrue(labelled > 0, "expected labelled " + type + " nodes from the fixture, got none");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Stage 1 must remain additive: the label is added, the property is not touched. Removing it is
|
||||
* stage 3, and only after every query has moved off it.
|
||||
*/
|
||||
@Test
|
||||
void typePropertyIsStillPresentAndAgreesWithTheLabel() {
|
||||
try (Session session = driver.session()) {
|
||||
long withoutProperty = session.run(
|
||||
"MATCH (n:AstNode {project: $p}) WHERE n.type IS NULL RETURN count(n) AS c",
|
||||
Map.of("p", PROJECT)).single().get("c").asLong();
|
||||
assertEquals(0, withoutProperty, "stage 1 keeps the type property — queries still read it");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The label must survive a re-ingest, which re-MERGEs onto the existing node rather than
|
||||
* creating a fresh one.
|
||||
*/
|
||||
@Test
|
||||
void labelSurvivesReIngest() {
|
||||
given().when().post("/api/projects/" + PROJECT + "/refresh").then().statusCode(200);
|
||||
try (Session session = driver.session()) {
|
||||
long mismatched = session.run(
|
||||
"MATCH (n:AstNode {project: $p}) WHERE NOT n.type IN labels(n) RETURN count(n) AS c",
|
||||
Map.of("p", PROJECT)).single().get("c").asLong();
|
||||
assertEquals(0, mismatched, "re-ingest must not strip or desynchronise the label");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -42,6 +42,7 @@ public final class CypherQueries {
|
||||
SET node.id = n.id, node.language = n.language, node.startLine = n.startLine,
|
||||
node.endLine = n.endLine, node.dataType = n.dataType, node.value = n.value
|
||||
SET node += n.properties
|
||||
SET node:$(n.type)
|
||||
""";
|
||||
|
||||
/**
|
||||
@@ -55,6 +56,7 @@ public final class CypherQueries {
|
||||
SET node.id = n.id, node.language = n.language,
|
||||
node.endLine = n.endLine, node.dataType = n.dataType, node.value = n.value
|
||||
SET node += n.properties
|
||||
SET node:$(n.type)
|
||||
""";
|
||||
|
||||
/**
|
||||
@@ -931,8 +933,8 @@ public final class CypherQueries {
|
||||
*/
|
||||
public static final String MODULE_HOP_OUT = """
|
||||
UNWIND $names AS n
|
||||
MATCH (m:AstNode {type: 'MODULE', name: n, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS]->(callee:AstNode {type: 'MODULE'})
|
||||
MATCH (m:MODULE {name: n, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS]->(callee:MODULE)
|
||||
WHERE coalesce(r.manualHidden, false) = false
|
||||
RETURN DISTINCT callee.name AS name
|
||||
""";
|
||||
@@ -955,8 +957,8 @@ public final class CypherQueries {
|
||||
*/
|
||||
public static final String MODULE_HOP_OUT_WIRING = """
|
||||
UNWIND $names AS n
|
||||
MATCH (m:AstNode {type: 'MODULE', name: n, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS|INJECTS|REFERENCES]->(callee:AstNode {type: 'MODULE'})
|
||||
MATCH (m:MODULE {name: n, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS|INJECTS|REFERENCES]->(callee:MODULE)
|
||||
WHERE coalesce(r.manualHidden, false) = false
|
||||
RETURN DISTINCT callee.name AS name
|
||||
""";
|
||||
@@ -967,9 +969,9 @@ public final class CypherQueries {
|
||||
* like the other call-ref queries (name/type/sourceFile/edgeKind/lineNos) so it reuses the same DTO.
|
||||
*/
|
||||
public static final String FUNCTION_CALLERS = """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(callee:AstNode {type: 'FUNCTION', name: $function})
|
||||
MATCH (caller:AstNode {type: 'FUNCTION'})-[r:CALLS]->(callee)
|
||||
MATCH (m:MODULE {name: $name, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(callee:FUNCTION {name: $function})
|
||||
MATCH (caller:FUNCTION)-[r:CALLS]->(callee)
|
||||
RETURN caller.name AS name, caller.type AS type, caller.sourceFile AS sourceFile,
|
||||
coalesce(r.callKind, 'PERFORM') AS edgeKind,
|
||||
collect({lineNo: r.lineNo, callSiteFile: coalesce(r.originFile, caller.sourceFile),
|
||||
@@ -2039,11 +2041,11 @@ public final class CypherQueries {
|
||||
AND ($type IS NULL OR n.type = $type)
|
||||
AND ($sourceFile IS NULL OR n.sourceFile = $sourceFile)
|
||||
AND ($module IS NULL OR EXISTS {
|
||||
MATCH (mod:AstNode {type: 'MODULE', name: $module, project: $project})
|
||||
MATCH (mod:MODULE {name: $module, project: $project})
|
||||
WHERE mod.sourceFile = n.sourceFile
|
||||
})
|
||||
WITH n, CASE WHEN $priorityModule IS NOT NULL AND EXISTS {
|
||||
MATCH (pm:AstNode {type: 'MODULE', name: $priorityModule, project: $project})
|
||||
MATCH (pm:MODULE {name: $priorityModule, project: $project})
|
||||
WHERE pm.sourceFile = n.sourceFile
|
||||
} THEN 0 ELSE 1 END AS pinRank
|
||||
RETURN n.id AS id, n.type AS type, n.name AS name, n.sourceFile AS sourceFile,
|
||||
@@ -2083,6 +2085,42 @@ public final class CypherQueries {
|
||||
MATCH (n)
|
||||
CALL { WITH n DETACH DELETE n } IN TRANSACTIONS OF $batchSize ROWS
|
||||
""";
|
||||
/**
|
||||
* Backfills the per-{@link NodeType} label onto nodes that predate it (stage 1 of the
|
||||
* label migration). {@code MERGE_NODES}/{@code MERGE_POSITIONAL_NODES} set it on every write, so
|
||||
* this only has to catch nodes an ingest has not touched since; it is a no-op once they all carry
|
||||
* one, which is why it is safe to run on every startup.
|
||||
*
|
||||
* <p><b>Why labels at all.</b> The type lived only in the {@code type} property, so every
|
||||
* expand-and-filter read it out of the property store. Profiled on {@code upms} (570k nodes,
|
||||
* 2.0M relationships), one representative traversal —
|
||||
* {@code (:AstNode{type:'MODULE',project})-[:CALLS]->(:AstNode{type:'MODULE'})} — cost
|
||||
* 1,598,264 dbHits, against 663,156 for the same pattern without the target's type filter:
|
||||
* <b>~935k dbHits, 58% of the query, spent reading one string property</b> (~36 hits per
|
||||
* candidate, since {@code type} sits in an 11-property chain). A label lives in the node record
|
||||
* and costs no property-store access at all.
|
||||
*
|
||||
* <p>Batched {@code IN TRANSACTIONS}: an unbatched {@code SET} over 570k nodes builds one
|
||||
* transaction state larger than the configured 2 GB Neo4j heap.
|
||||
*/
|
||||
public static final String BACKFILL_TYPE_LABELS = """
|
||||
MATCH (n:AstNode)
|
||||
WHERE n.type IS NOT NULL AND NOT n.type IN labels(n)
|
||||
CALL { WITH n SET n:$(n.type) } IN TRANSACTIONS OF $batchSize ROWS
|
||||
""";
|
||||
|
||||
/**
|
||||
* Verification companion to {@link #BACKFILL_TYPE_LABELS}: how many nodes still lack their type
|
||||
* label. Read back after the backfill so a partial run reports itself, rather than being inferred
|
||||
* from a write counter that reads {@code 0} both when nothing needed doing and when nothing was
|
||||
* done.
|
||||
*/
|
||||
public static final String COUNT_NODES_MISSING_TYPE_LABEL = """
|
||||
MATCH (n:AstNode)
|
||||
WHERE n.type IS NOT NULL AND NOT n.type IN labels(n)
|
||||
RETURN count(n) AS missing
|
||||
""";
|
||||
|
||||
/**
|
||||
* Schema statements (indexes/constraints) applied once at startup. Each is run as
|
||||
* its own statement, since Neo4j requires schema operations outside of a
|
||||
@@ -2108,7 +2146,14 @@ public final class CypherQueries {
|
||||
// Item 82: manual dynamic-CALLNAT overrides are keyed by (project, originFile, lineNo);
|
||||
// the apply/list/reset queries seek by project. Not on the :AstNode label on purpose, so
|
||||
// DELETE_PROJECT_NODES (which matches :AstNode only) leaves overrides intact across a refresh.
|
||||
"CREATE INDEX dyn_call_override_project IF NOT EXISTS FOR (o:DynamicCallOverride) ON (o.project, o.originFile, o.lineNo)"
|
||||
"CREATE INDEX dyn_call_override_project IF NOT EXISTS FOR (o:DynamicCallOverride) ON (o.project, o.originFile, o.lineNo)",
|
||||
// Item 111b: a query anchored as (m:MODULE {project, name}) cannot use the :AstNode indexes
|
||||
// — an index serves exactly one label. Without these, swapping the property filter for a
|
||||
// label would turn the anchor lookup from an index seek into a label scan and make the
|
||||
// migration a regression at the very point it is meant to help. Added for the two labels
|
||||
// the migrated hot paths anchor on; the rest follow as their queries move.
|
||||
"CREATE INDEX module_project_name IF NOT EXISTS FOR (n:MODULE) ON (n.project, n.name)",
|
||||
"CREATE INDEX function_project_name IF NOT EXISTS FOR (n:FUNCTION) ON (n.project, n.name)"
|
||||
);
|
||||
|
||||
/**
|
||||
@@ -2211,13 +2256,17 @@ public final class CypherQueries {
|
||||
// from inside a subroutine/method has its CALLS edge originating at that FUNCTION node;
|
||||
// walking CONTAINS*0..1 back to the owning MODULE attributes the call to the module — exactly
|
||||
// as `callees` anchors its source side with (m)-[:CONTAINS*0..1]->(source). `collect(DISTINCT
|
||||
// ...)` collapses the one-row-per-call-site duplication, and requiring `callerModule.type =
|
||||
// 'MODULE'` keeps the view module-only. Without this roll-up the default view leaked
|
||||
// ...)` collapses the one-row-per-call-site duplication, and the `:MODULE` label on
|
||||
// `callerModule` keeps the view module-only. Without this roll-up the default view leaked
|
||||
// FUNCTION-typed callers and duplicated hot utilities one row per call site (audit Bug A);
|
||||
// `callerModule <> m` drops the module's self-loop, which `callees` never mirrors.
|
||||
//
|
||||
// Item 111b: anchored on the `:MODULE` label rather than a `type: 'MODULE'` property. The
|
||||
// type property is still written and still correct — this is purely about not reading it
|
||||
// out of the property store, where it costs ~36 dbHits per candidate node.
|
||||
return """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH (callerModule:AstNode {type: 'MODULE', project: $project})
|
||||
MATCH (m:MODULE {name: $name, project: $project})
|
||||
MATCH (callerModule:MODULE {project: $project})
|
||||
-[:CONTAINS*0..1]->(source:AstNode)
|
||||
-[r:CALLS|EXTENDS|IMPLEMENTS|INJECTS|REFERENCES]->(m)
|
||||
WHERE callerModule <> m
|
||||
@@ -2233,7 +2282,7 @@ public final class CypherQueries {
|
||||
// `target <> m` and `caller <> m` drop the main body's self-loop (MODULE performing its own
|
||||
// subroutines), which would otherwise surface the module as its own caller.
|
||||
return """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH (m:MODULE {name: $name, project: $project})
|
||||
MATCH (caller:AstNode)-[r:CALLS|EXTENDS|IMPLEMENTS|INJECTS|REFERENCES]->(target:AstNode)
|
||||
WHERE (m)-[:CONTAINS]->(target) AND target <> m AND caller <> m
|
||||
RETURN caller.name AS name, caller.type AS type, caller.sourceFile AS sourceFile,
|
||||
@@ -2386,13 +2435,15 @@ public final class CypherQueries {
|
||||
*/
|
||||
public static String callees(@Nullable String scope, boolean resolveInterfaces) {
|
||||
String scopeFilter = switch (scope == null ? "" : scope.toLowerCase(Locale.ROOT)) {
|
||||
case "external" -> "AND callee.type = 'MODULE'";
|
||||
case "internal" -> "AND callee.type = 'FUNCTION'";
|
||||
// Item 111b: `callee:MODULE` instead of `callee.type = 'MODULE'` — the filter runs on
|
||||
// every expanded candidate, which is exactly where the property read hurts most.
|
||||
case "external" -> "AND callee:MODULE";
|
||||
case "internal" -> "AND callee:FUNCTION";
|
||||
default -> "";
|
||||
};
|
||||
if (!resolveInterfaces) {
|
||||
return """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH (m:MODULE {name: $name, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(source:AstNode)-[r:CALLS|EXTENDS|IMPLEMENTS|INJECTS|REFERENCES]->(callee:AstNode)
|
||||
WHERE 1=1 %s
|
||||
AND coalesce(r.manualHidden, false) = false
|
||||
@@ -2407,7 +2458,7 @@ public final class CypherQueries {
|
||||
""".formatted(scopeFilter);
|
||||
}
|
||||
return """
|
||||
MATCH (m:AstNode {type: 'MODULE', name: $name, project: $project})
|
||||
MATCH (m:MODULE {name: $name, project: $project})
|
||||
MATCH (m)-[:CONTAINS*0..1]->(source:AstNode)-[r:CALLS|EXTENDS|IMPLEMENTS|INJECTS|REFERENCES]->(callee:AstNode)
|
||||
WHERE 1=1 %s
|
||||
AND coalesce(r.manualHidden, false) = false
|
||||
|
||||
@@ -1714,11 +1714,41 @@ public class GraphRepository {
|
||||
for (String statement : CypherQueries.SCHEMA_STATEMENTS) {
|
||||
session.run(statement).consume();
|
||||
}
|
||||
backfillTypeLabels(session);
|
||||
}
|
||||
return null;
|
||||
}).replaceWithVoid();
|
||||
}
|
||||
|
||||
/**
|
||||
* Gives nodes written before the per-{@link com.agenticcode.parsercore.ast.model.NodeType} label
|
||||
* existed their label, so queries need not care whether a node predates the migration. Runs
|
||||
* outside {@code executeWrite} because {@code CALL { … } IN TRANSACTIONS} manages its own
|
||||
* transactions (same reason as {@link #deleteNodesBatched}).
|
||||
*
|
||||
* <p>Cheap once the graph is migrated — the {@code NOT n.type IN labels(n)} filter matches
|
||||
* nothing and no write happens.
|
||||
*
|
||||
* <p><b>Always logs, and verifies rather than trusting the write counter.</b> The first version
|
||||
* logged only when {@code labelsAdded() > 0} and stayed silent on a graph that was already fully
|
||||
* labelled — indistinguishable from "the backfill never ran", which is exactly the question this
|
||||
* log line exists to answer. It now re-counts the stragglers afterwards, so an interrupted or
|
||||
* partial run is visible as a non-zero remainder instead of having to be inferred.
|
||||
*/
|
||||
private void backfillTypeLabels(Session session) {
|
||||
SummaryCounters counters = session.run(CypherQueries.BACKFILL_TYPE_LABELS,
|
||||
Map.of("batchSize", deleteBatchSize)).consume().counters();
|
||||
long remaining = session.run(CypherQueries.COUNT_NODES_MISSING_TYPE_LABEL)
|
||||
.single().get("missing").asLong();
|
||||
if (remaining > 0) {
|
||||
LOG.warnf("Type-label backfill added %d labels but %d nodes still lack theirs — "
|
||||
+ "the run did not complete; it retries on the next startup",
|
||||
counters.labelsAdded(), remaining);
|
||||
} else {
|
||||
LOG.infof("Type labels complete (%d added this startup)", counters.labelsAdded());
|
||||
}
|
||||
}
|
||||
|
||||
public Uni<Boolean> projectExists(String name) {
|
||||
return Uni.createFrom().item(() -> {
|
||||
try (Session session = driver.session()) {
|
||||
|
||||
@@ -16,10 +16,14 @@ services:
|
||||
# Deliberately raised 512M -> 1G alongside the smaller heap: the store is 2.0 GB, so a
|
||||
# bigger page cache offsets the tighter heap instead of pushing the load onto disk.
|
||||
NEO4J_server_memory_pagecache_size: "1G"
|
||||
# Hard ceiling: heap 2G + pagecache 1G + ~1G for metaspace, direct buffers and GC overhead.
|
||||
# Deliberately not tighter — an OOM-kill mid-refresh would leave the graph half-updated, which
|
||||
# is far worse than a GB of headroom. Still well under the 6.17 GiB measured without a cap.
|
||||
mem_limit: 4g
|
||||
# Return committed-but-unused heap to the OS while idle instead of sitting on it. Without this
|
||||
# the JVM held everything it had ever needed: 6.17 GiB an hour after a refresh had finished.
|
||||
NEO4J_server_jvm_additional: "-XX:G1PeriodicGCInterval=300000"
|
||||
# Hard ceiling: heap 2G + pagecache 1G + metaspace, direct buffers and GC overhead.
|
||||
# Raised 4g -> 5g after the verification run peaked at exactly 4096 of 4096 MB — no OOM-kill,
|
||||
# but zero reserve. An OOM-kill mid-refresh leaves the graph half-updated, which is far worse
|
||||
# than a spare GB. Still well under the 6.17 GiB measured with no cap at all.
|
||||
mem_limit: 5g
|
||||
volumes:
|
||||
- neo4j-data:/data
|
||||
healthcheck:
|
||||
@@ -45,12 +49,18 @@ services:
|
||||
# compose replaces the image's, it does not append, so the log-manager property would be
|
||||
# silently dropped if it were left in the Dockerfile.
|
||||
#
|
||||
# -Xmx3g against a measured heap peak of 2772 MB during the upms deep refresh (45 finalize
|
||||
# samples, never above 3 GB). Ergonomics would otherwise allow 7956 MB and the process kept
|
||||
# 4.91 GiB resident an hour after the refresh ended.
|
||||
JDK_JAVA_OPTIONS: "-Xmx3g -Djava.util.logging.manager=org.jboss.logmanager.LogManager"
|
||||
# Heap 3G + metaspace/code cache/direct buffers. Peak RSS measured during the refresh was 5.0 GB
|
||||
# with an uncapped heap; with -Xmx3g this ceiling leaves room without allowing that again.
|
||||
# -Xmx2500m. Ergonomics would allow 7956 MB, and the process then kept 4.91 GiB resident an
|
||||
# hour after a refresh had ended. Sizing history, because the first cap was too tight: a deep
|
||||
# *refresh* peaked at 1593 MB, which suggested 2 GB — but a `project recreate --deep` (heavier:
|
||||
# ingests from empty) then peaked at 1924 MB, i.e. 94% of that cap. 2500m restores ~25% headroom
|
||||
# while keeping nearly all of the reduction.
|
||||
# G1PeriodicGCInterval returns committed-but-unused heap to the OS while idle.
|
||||
JDK_JAVA_OPTIONS: >-
|
||||
-Xmx2500m
|
||||
-XX:G1PeriodicGCInterval=300000
|
||||
-Djava.util.logging.manager=org.jboss.logmanager.LogManager
|
||||
# Heap 2G + metaspace/code cache/direct buffers. Measured RSS peak was 3472 MB against a 3 GB
|
||||
# heap cap, i.e. ~1.9 GB non-heap, so the ceiling stays at 4g even though the heap shrank.
|
||||
mem_limit: 4g
|
||||
volumes:
|
||||
# Mounted at the same absolute host path so project roots registered via
|
||||
|
||||
@@ -7,6 +7,22 @@ schema — only what is produced today).
|
||||
`CALLS` edges carry a language-aware `callKind` property (see the `CallKind` enum in
|
||||
`ac-parser-core`), surfaced as `edgeKind` by the `callers`/`callees` queries.
|
||||
|
||||
## Neo4j labels (item 111a, 2026-08-05)
|
||||
|
||||
Every node carries **two** labels: `AstNode`, plus its `NodeType` name — `:MODULE`, `:FUNCTION`,
|
||||
`:VARIABLE`, `:DATA_STRUCTURE`, `:DB_TABLE`, `:CONSTANT`, `:FIELD`, `:DB_ACCESS`, `:WORKFILE`,
|
||||
`:WORKFILE_ACCESS`, `:CONTROL_FLOW`, `:PAYLOAD_FIELD`. Written by both persist paths and backfilled
|
||||
onto pre-existing nodes at startup.
|
||||
|
||||
The type is **also still a `type` property**, and every query currently reads that property. The
|
||||
label is the migration target, not yet the source of truth: filtering on `type` out of the property
|
||||
store cost ~935k of 1.6M dbHits (58%) in a profiled `upms` traversal, because the property sits in
|
||||
an 11-property chain, whereas a label lives in the node record. Query sites move to `:LABEL` one at
|
||||
a time under measurement (roadmap 111b); the property is dropped only once none read it (111c).
|
||||
|
||||
Writing your own Cypher: prefer `MATCH (n:MODULE {project: $p})` over
|
||||
`MATCH (n:AstNode {type: 'MODULE', project: $p})` — both are correct today, the first is cheaper.
|
||||
|
||||
## Java — `JavaParser`
|
||||
|
||||
```mermaid
|
||||
|
||||
@@ -75,6 +75,146 @@ bottleneck — and 25 — batch persist, found already implemented — completed
|
||||
before. The parked "parallel parse phase" idea was implemented 2026-07-18 (item 24) — see
|
||||
`x-docs/features.md`. **No open items remain in this track.**)*
|
||||
|
||||
- [x] **111a. `NodeType` as a Neo4j label, stage 1: written on every persist** (done 2026-08-05)
|
||||
|
||||
**Measured problem.** The node type lived only in the `type` property, so every expand-and-filter
|
||||
read it out of the property store. Profiled on `upms` (570,739 nodes, 2,002,935 relationships):
|
||||
|
||||
| query | dbHits | rows |
|
||||
|---|---|---|
|
||||
| `(:AstNode{type:'MODULE',project})-[:CALLS]->(:AstNode{type:'MODULE'})` | 1,598,264 | 5,384 |
|
||||
| same, without the target's `type` filter | 663,156 | 25,843 |
|
||||
|
||||
**~935k dbHits — 58% of the query — spent reading one string property**, ~36 hits per candidate
|
||||
node because `type` sits in an 11-property chain. A label lives in the node record and costs no
|
||||
property-store access. Of the 200 `type: '…'` occurrences in `CypherQueries`, **106 filter on
|
||||
`type` without binding `name`**, and **none** bind both — so the composite index
|
||||
`(project, type, name)` is never used with all three columns, and the win is in traversal
|
||||
filtering, not in index seeks.
|
||||
|
||||
**Delivered (stage 1, deliberately additive).** `SET node:$(n.type)` in both persist paths
|
||||
(`MERGE_NODES`, `MERGE_POSITIONAL_NODES` — dynamic labels verified available on Neo4j 5.26.27),
|
||||
plus `BACKFILL_TYPE_LABELS` run from `ensureSchema()`, batched `IN TRANSACTIONS` so 570k nodes do
|
||||
not build one transaction state larger than the 2 GB Neo4j heap. Labels take no part in the MERGE
|
||||
identity, so merge semantics are unchanged. **The `type` property stays**, so none of the ~200
|
||||
queries change behaviour and nothing has to migrate at once. Covered by `TypeLabelIT`.
|
||||
|
||||
**Not yet done — stage 1 alone buys nothing.** It only creates the precondition:
|
||||
- **111c:** drop the `type` property and the four `AstNode` collection indexes once no query reads
|
||||
it (−9 chars/node).
|
||||
|
||||
- [x] **111b. Hot-path queries anchored on the label instead of the `type` property**
|
||||
(done 2026-08-05, builds on 111a)
|
||||
|
||||
Migrated in `CypherQueries`: `callers` (both scopes), `callees` (both branches **and** its
|
||||
`scope` filter, which runs on every expanded candidate), `MODULE_HOP_OUT` /
|
||||
`MODULE_HOP_OUT_WIRING` (the call-tree traversal), `FUNCTION_CALLERS`, and the two module anchors
|
||||
in `SEARCH_IDENTIFIER`. Its `$type` filter stays a property comparison on purpose — it is a bound
|
||||
parameter, not a literal, so it cannot become a static label.
|
||||
|
||||
**Two new indexes were required, not optional.** A Neo4j index serves exactly one label, so
|
||||
`(m:MODULE {project, name})` cannot use the `:AstNode` indexes. Without
|
||||
`module_project_name` / `function_project_name` the anchor lookup would degrade from an index seek
|
||||
to a label scan — turning the migration into a regression at the very point it is meant to help.
|
||||
|
||||
~158 `type: '…'` literals remain in the enrichment queries; they migrate incrementally, each with
|
||||
its own measurement.
|
||||
|
||||
**Baseline captured before deploying** (upms, best of 3 per endpoint, version 160):
|
||||
|
||||
| module | endpoint | before |
|
||||
|---|---|---|
|
||||
| WGEAGB0S | `callers` / `callees` | 24 ms / 14 ms |
|
||||
| WGEAGB0S | `call-tree?depth=3` / `depth=5` | 266 ms / 312 ms |
|
||||
| ZINERR01 (1689 callers) | `callers` | 329 ms |
|
||||
| YFRAMN04 (916 callers) | `callers` / `call-tree?depth=5` | 335 ms / 262 ms |
|
||||
|
||||
**Measured after deploying (version 162) — the performance case did not hold up.**
|
||||
|
||||
| module | endpoint | before | after |
|
||||
|---|---|---|---|
|
||||
| WGEAGB0S | `callers` / `callees` | 24 / 14 ms | 21 / 16 ms |
|
||||
| WGEAGB0S | `call-tree?depth=3` / `depth=5` | 266 / 312 ms | 255 / 292 ms |
|
||||
| ZINERR01 | `callers` | 329 ms | 323 ms |
|
||||
| YFRAMN04 | `callers` / `call-tree?depth=5` | 335 / 262 ms | 277 / 141 ms |
|
||||
|
||||
At the Cypher level, where dbHits are deterministic and HTTP noise is absent:
|
||||
|
||||
| query | before | after | change |
|
||||
|---|---|---|---|
|
||||
| `callers` YFRAMN04 | 16,202 dbHits | 14,369 | **−11%** |
|
||||
| module hop (call-tree core), 4 seeds | 1,864 dbHits | 1,850 | **−0.8%** |
|
||||
|
||||
**Why the 15.8× microbenchmark did not transfer.** It scanned every module in the project and
|
||||
filtered the expansion; the real queries seek one module by name and expand over hundreds of edges.
|
||||
There are simply almost no property reads left to eliminate. The `YFRAMN04 call-tree` 1.9× is not
|
||||
attributable to the migration — the module-hop core it is built from improved by 0.8%. A 12-run
|
||||
repeat of the one apparently slower endpoint (`WGEAGB0S/callers`) gave min 21 ms / median 43 /
|
||||
max 66: no regression, just a measurement dominated by noise.
|
||||
|
||||
**Kept despite this**, on a different justification than the one it was proposed under: it is a
|
||||
small consistent improvement and no regression, and — the actual reason — **111c cannot happen
|
||||
without it**. Dropping the `type` property requires that no query reads it. The remaining value of
|
||||
111a/b is the storage reduction 111c unlocks, not query speed.
|
||||
|
||||
**Lesson recorded deliberately:** the pre-implementation benchmark was chosen for convenience, not
|
||||
for resemblance to the production query shape, and overstated the benefit by more than two orders
|
||||
of magnitude. Benchmark the query the code actually runs. 295/295 ITs pass.
|
||||
|
||||
**Write cost — measured 2026-08-05, no regression.** A `project recreate upms --deep` with the
|
||||
label writes in place took **541 s** for the parse/persist phase (6311 files), against **853 s**
|
||||
for the same phase without them. Not a like-for-like comparison — a recreate creates nodes rather
|
||||
than merging onto existing ones and skips per-file reconciliation, so it is expected to be faster —
|
||||
but there is no sign of the slowdown that would have forced a revert.
|
||||
|
||||
**Payoff confirmed on the real graph — bigger than estimated.** Same graph, same query, same result
|
||||
(5381 rows), only the filter style differs:
|
||||
|
||||
| | dbHits | time |
|
||||
|---|---|---|
|
||||
| `(:AstNode{type:'MODULE',project})-[:CALLS]->(:AstNode{type:'MODULE'})` | 1,601,071 | 512 ms |
|
||||
| `(:MODULE{project})-[:CALLS]->(:MODULE)` | **101,120** | **25 ms** |
|
||||
|
||||
15.8× fewer dbHits, 20× faster — **but do not read this as the payoff of the migration. It is not.**
|
||||
This query starts from *every* module in the project and expands, so the property filter is applied
|
||||
to hundreds of thousands of candidates. The real API queries anchor on a *single* module by name via
|
||||
an index seek and expand from there, over hundreds of edges rather than hundreds of thousands. See
|
||||
111b for what the migration actually delivered on those (11% and 0.8%, not 15×). The number above
|
||||
is a property of this microbenchmark, not of the codebase.
|
||||
|
||||
- [ ] **111d. `sourceFile` and the `id` UUID are the bulk of the store — intern them**
|
||||
(found 2026-08-05 while sizing container memory)
|
||||
|
||||
`neostore.propertystore.db.strings` is **385 MB**, the largest file in the 2.0 GB store (plus
|
||||
321 MB property store). Average string bytes per node:
|
||||
|
||||
| property | ⌀ chars | note |
|
||||
|---|---|---|
|
||||
| `sourceFile` | 59 | only **11,080 distinct values across 570,739 nodes** — every path stored ~51× |
|
||||
| `id` | 36 | a UUID the docs themselves call meaningless across ingest generations |
|
||||
| `name` | 13 | genuine |
|
||||
| `language` / `project` | 6 / 4 | constant per project |
|
||||
| `value` / `dataType` | 8 / 4 | genuine |
|
||||
|
||||
`sourceFile` + `id` are **95 of ~139 chars per node (68%) and pure redundancy**. Interning
|
||||
`sourceFile` (a `(:SourceFile {path})` node, or a per-project integer index) and dropping the `id`
|
||||
UUID would also retire the `ast_node_id_unique` constraint, which is maintained across 570k writes
|
||||
per refresh for an identifier with no cross-generation meaning.
|
||||
|
||||
Shortening the property chain compounds with 111a/b: the ~36 dbHits to reach `type` are a function
|
||||
of chain length, so removing properties speeds up reads of **all** the others.
|
||||
|
||||
Invasive — touches the persist path, most Cypher queries, the response DTOs and (for `id`) the
|
||||
public `/nodes/{id}` API and the web UI. Do after 111b/c.
|
||||
|
||||
- [ ] **111e. Is a persisted `CONTROL_FLOW` node per statement worth it?** (raised 2026-08-05)
|
||||
|
||||
147,483 `CONTROL_FLOW` nodes = **26% of all nodes**, each carrying the same ~139 chars of mostly
|
||||
redundant metadata. Sources are always readable to a human or agent that knows the location, so the
|
||||
question is which queries genuinely need persisted statement-level nodes rather than a location plus
|
||||
an on-demand re-parse. Largest single reduction available, but open-ended: what depends on them has
|
||||
to be established first.
|
||||
|
||||
## Known bugs
|
||||
|
||||
- [x] **107. Every module endpoint answers `200` with an empty shell for a module that does not exist —
|
||||
|
||||
Reference in New Issue
Block a user