Skip to content

perf: reduce query latency regression from 3.1.4 to 3.3.0 #528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlos-alm merged 5 commits into from
Mar 19, 2026
Merged

perf: reduce query latency regression from 3.1.4 to 3.3.0 #528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50ec4a0
perf: reduce query latency regression from 3.1.4 → 3.3.0
carlos-alm Mar 19, 2026
1468ef1
fix: return structuredClone from config cache and guard benchmark db …
carlos-alm Mar 19, 2026
7491363
fix: install @huggingface/transformers in npm-mode benchmark workers
carlos-alm Mar 19, 2026
1f9e2e5
fix: clone config on cache-miss paths to prevent cache corruption (#528)
carlos-alm Mar 19, 2026
55a9f36
Merge branch 'main' into perf/query-latency-regression
carlos-alm Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions scripts/lib/bench-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,27 @@ export async function resolveBenchmarkSource() {
console.error(`Warning: failed to install native package: ${err.message}`);
}

// @huggingface/transformers is a devDependency (lazy-loaded for embeddings).
// It is not installed as a transitive dep in npm mode, so install it
// explicitly so the embedding benchmark workers can import it.
try {
const localPkg = JSON.parse(
fs.readFileSync(path.resolve(path.dirname(new URL(import.meta.url).pathname.replace(/^\/([A-Z]:)/, '$1')), '..', '..', 'package.json'), 'utf8'),
);
const hfVersion = localPkg.devDependencies?.['@huggingface/transformers'];
if (hfVersion) {
console.error(`Installing @huggingface/transformers@${hfVersion} for embedding benchmarks...`);
execFileSync('npm', ['install', `@huggingface/transformers@${hfVersion}`, '--no-audit', '--no-fund', '--no-save'], {
cwd: tmpDir,
stdio: 'pipe',
timeout: 120_000,
});
console.error('Installed @huggingface/transformers');
}
} catch (err) {
console.error(`Warning: failed to install @huggingface/transformers: ${err.message}`);
}

const srcDir = path.join(pkgDir, 'src');

if (!fs.existsSync(srcDir)) {
Expand Down
33 changes: 31 additions & 2 deletions scripts/query-benchmark.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,32 @@ function round1(n) {
return Math.round(n * 10) / 10;
}

// Pinned hub targets — stable function names that exist across versions.
// Auto-selecting the most-connected node makes version-to-version comparison
// meaningless when barrel/type files get added or removed.
const PINNED_HUB_CANDIDATES = ['buildGraph', 'openDb', 'loadConfig'];

function selectTargets() {
const db = new Database(dbPath, { readonly: true });
try {

// Try pinned candidates first for a stable hub across versions
let hub = null;
for (const candidate of PINNED_HUB_CANDIDATES) {
const row = db
.prepare(
`SELECT n.name FROM nodes n
JOIN edges e ON e.source_id = n.id OR e.target_id = n.id
WHERE n.name = ? AND n.file NOT LIKE '%test%' AND n.file NOT LIKE '%spec%'
LIMIT 1`,
)
.get(candidate);
if (row) {
hub = row.name;
break;
}
}

const rows = db
.prepare(
`SELECT n.name, COUNT(e.id) AS cnt
Expand All @@ -123,14 +147,19 @@ function selectTargets() {
ORDER BY cnt DESC`,
)
.all();
db.close();

if (rows.length === 0) throw new Error('No nodes with edges found in graph');

const hub = rows[0].name;
// Fall back to most-connected if no pinned candidate found
if (!hub) hub = rows[0].name;

const mid = rows[Math.floor(rows.length / 2)].name;
const leaf = rows[rows.length - 1].name;
return { hub, mid, leaf };

} finally {
db.close();
}
}

function benchDepths(fn, name, depths) {
Expand Down
9 changes: 9 additions & 0 deletions scripts/update-embedding-report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@ if (arg) {
}
const entry = JSON.parse(jsonText);

// Guard: reject empty benchmark results (all workers crashed or no symbols indexed)
if (!entry.symbols || !entry.models || Object.keys(entry.models).length === 0) {
console.error(
`Embedding benchmark produced empty results (symbols=${entry.symbols}, models=${Object.keys(entry.models || {}).length}). ` +
'Skipping report update to avoid overwriting valid data. Check benchmark worker logs.',
);
process.exit(1);
}

// ── Paths ────────────────────────────────────────────────────────────────
const reportPath = path.join(root, 'generated', 'benchmarks', 'EMBEDDING-BENCHMARKS.md');

Expand Down
20 changes: 18 additions & 2 deletions src/domain/analysis/impact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ import { findMatchingNodes } from './symbol-lookup.js';

const INTERFACE_LIKE_KINDS = new Set(['interface', 'trait']);

/**
* Check whether the graph contains any 'implements' edges.
* Cached per db handle so the query runs at most once per connection.
*/
const _hasImplementsCache = new WeakMap();
function hasImplementsEdges(db) {
if (_hasImplementsCache.has(db)) return _hasImplementsCache.get(db);
const row = db.prepare("SELECT 1 FROM edges WHERE kind = 'implements' LIMIT 1").get();
const result = !!row;
_hasImplementsCache.set(db, result);
return result;
}

/**
* BFS traversal to find transitive callers of a node.
* When an interface/trait node is encountered (either as the start node or
Expand All @@ -40,14 +53,17 @@ export function bfsTransitiveCallers(
startId,
{ noTests = false, maxDepth = 3, includeImplementors = true, onVisit } = {},
) {
// Skip all implementor lookups when the graph has no implements edges
const resolveImplementors = includeImplementors && hasImplementsEdges(db);

const visited = new Set([startId]);
const levels = {};
let frontier = [startId];

// Seed: if start node is an interface/trait, include its implementors at depth 1.
// Implementors go into a separate list so their callers appear at depth 2, not depth 1.
const implNextFrontier = [];
if (includeImplementors) {
if (resolveImplementors) {
const startNode = findNodeById(db, startId);
if (startNode && INTERFACE_LIKE_KINDS.has(startNode.kind)) {
const impls = findImplementors(db, startId);
Expand Down Expand Up @@ -88,7 +104,7 @@ export function bfsTransitiveCallers(

// If a caller is an interface/trait, also pull in its implementors
// Implementors are one extra hop away, so record at d+1
if (includeImplementors && INTERFACE_LIKE_KINDS.has(c.kind)) {
if (resolveImplementors && INTERFACE_LIKE_KINDS.has(c.kind)) {
const impls = findImplementors(db, c.id);
for (const impl of impls) {
if (!visited.has(impl.id) && (!noTests || !isTestFile(impl.file))) {
Expand Down
26 changes: 23 additions & 3 deletions src/infrastructure/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,19 @@ export const DEFAULTS = {
},
};

// Per-cwd config cache — avoids re-reading the config file on every query call.
// The config file rarely changes within a single process lifetime.
const _configCache = new Map();

/**
* Load project configuration from a .codegraphrc.json or similar file.
* Returns merged config with defaults.
* Returns merged config with defaults. Results are cached per cwd.
*/
export function loadConfig(cwd) {
cwd = cwd || process.cwd();
const cached = _configCache.get(cwd);
if (cached) return structuredClone(cached);

for (const name of CONFIG_FILES) {
const filePath = path.join(cwd, name);
if (fs.existsSync(filePath)) {
Expand All @@ -148,13 +155,26 @@ export function loadConfig(cwd) {
merged.query.excludeTests = Boolean(config.excludeTests);
}
delete merged.excludeTests;
return resolveSecrets(applyEnvOverrides(merged));
const result = resolveSecrets(applyEnvOverrides(merged));
_configCache.set(cwd, structuredClone(result));
return result;
} catch (err) {
debug(`Failed to parse config ${filePath}: ${err.message}`);
}
}
}
return resolveSecrets(applyEnvOverrides({ ...DEFAULTS }));
const defaults = resolveSecrets(applyEnvOverrides({ ...DEFAULTS }));
_configCache.set(cwd, structuredClone(defaults));
return defaults;
Comment on lines +158 to +168
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cache miss returns un-cloned reference

The structuredClone guard was applied to cache hits (line 144), but both cache-miss return paths hand the caller the exact same object that was just written into _configCache. A caller mutating the returned config on first use will corrupt every subsequent cache hit from that cwd.

The fix from the previous thread — "loadConfig() now returns structuredClone(cached) on cache hits" — is incomplete; the same protection needs to apply at initial population. Either store a clone in the cache and return the original, or return a clone at the end of each miss path:

Suggested change
const result = resolveSecrets(applyEnvOverrides(merged));
_configCache.set(cwd, result);
return result;
} catch (err) {
debug(`Failed to parse config ${filePath}: ${err.message}`);
}
}
}
return resolveSecrets(applyEnvOverrides({ ...DEFAULTS }));
const defaults = resolveSecrets(applyEnvOverrides({ ...DEFAULTS }));
_configCache.set(cwd, defaults);
return defaults;
const result = resolveSecrets(applyEnvOverrides(merged));
_configCache.set(cwd, structuredClone(result));
return result;

And the defaults path needs the same treatment:

  const defaults = resolveSecrets(applyEnvOverrides({ ...DEFAULTS }));
  _configCache.set(cwd, structuredClone(defaults));
  return defaults;

Without this, the invariant that "the cache stores a pristine copy that no external caller can touch" only holds after the second call to loadConfig(cwd), not the first.

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — both cache-miss paths now store structuredClone(result) in the cache and return the original object, ensuring callers can never mutate the cached copy.

}

/**
* Clear the config cache. Intended for long-running processes that need to
* pick up on-disk config changes, and for test isolation when tests share
* the same cwd.
*/
export function clearConfigCache() {
_configCache.clear();
}
Comment on lines +176 to 178
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 clearConfigCache exported but never imported anywhere

The PR description says this is exported "for test use," but a search across the whole codebase shows it is defined here and not imported or called anywhere — including in tests/unit/config.test.js, which achieves test isolation via unique os.tmpdir() directories instead.

An unused export is harmless, but it can mislead future contributors into thinking tests or callers rely on it. If the intent is to provide a safety valve for long-running processes that detect config-file changes on disk, a brief inline comment explaining that use case (or a corresponding test) would clarify the intent.

Copy link
Copy Markdown
Contributor Author

@carlos-alm carlos-alm Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — updated the JSDoc to clarify the intended use cases: long-running processes that need to detect on-disk config changes, and test isolation when tests share the same cwd.


const ENV_LLM_MAP = {
Expand Down
Loading