Skip to content

perf(native): raise native edge-building threshold to smallFilesThreshold#940

Merged
carlos-alm merged 2 commits intomainfrom
perf/raise-native-edge-threshold
Apr 16, 2026
Merged

perf(native): raise native edge-building threshold to smallFilesThreshold#940
carlos-alm merged 2 commits intomainfrom
perf/raise-native-edge-threshold

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Raise the native import/call edge-building threshold from hardcoded > 3 to ctx.config.build.smallFilesThreshold (default 5)
  • Small incremental builds (≤5 files) now use the JS edge path, avoiding ~13ms of napi-rs marshaling overhead
  • Aligns with the same threshold used for insertNodes, structure, and other pipeline stages

Closes #936

Test plan

  • Integration tests pass (564/564)
  • Verify with codegraph build --engine native --timing on a 5-file incremental — edges phase should drop from ~42ms to ~29ms
  • Verify full builds and large incrementals still use the native edge path

…hold

The hardcoded > 3 threshold for native import/call edge building
triggered on 5-file incremental builds, but the napi-rs marshaling
cost (~13ms) exceeded Rust computation savings at that scale.

Raise the threshold to use ctx.config.build.smallFilesThreshold
(default 5) so small incremental builds use the JS edge path,
aligning with the same threshold used for insertNodes and structure.

Closes #936

Impact: 1 functions changed, 5 affected
@claude
Copy link
Copy Markdown

claude bot commented Apr 16, 2026

Claude finished @carlos-alm's task —— View job


Reviewing PR #940: perf(native): raise native edge-building threshold to smallFilesThreshold

Review Progress

  • Phase 1: Root Cause Analysis - understand the problem being solved
  • Phase 2: Configuration Change Detection - check for high-risk changes
  • Phase 3: Test Integrity - verify test quality
  • Phase 4: Code Quality Review - analyze implementation
  • Phase 5: Backlog Compliance - rate against BACKLOG.md criteria
  • Final Assessment and Rating

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Performance optimization for small incremental builds. The hardcoded > 3 threshold was causing 5-file incremental builds to use the native Rust edge-building path, but the napi-rs marshaling overhead (~13ms) exceeded the actual computation savings at this scale.

Why This Approach: The change replaces the hardcoded threshold with ctx.config.build.smallFilesThreshold (default 5), which aligns with thresholds used elsewhere in the codebase for similar performance decisions (insertNodes, structure stages).

Root Cause Assessment: ✅ This appears to be a legitimate performance optimization addressing real overhead, not symptom hiding.

Phase 2: Configuration Change Detection ✅

HIGH-RISK CHANGES: None detected.

  • ✅ No linting rules disabled or relaxed
  • ✅ No test timeouts increased or coverage thresholds lowered
  • ✅ No build configuration or module resolution modified
  • ✅ No TypeScript strict mode flags disabled

CONFIGURATION ANALYSIS: The change uses an existing configuration value (ctx.config.build.smallFilesThreshold = 5) instead of introducing new config. This follows the CLAUDE.md guideline: "always add them to DEFAULTS and wire them through config — never introduce new hardcoded magic numbers".

Phase 3: Test Integrity ✅

RED FLAGS: None detected.

  • ✅ No test files modified
  • ✅ No assertions removed or weakened
  • ✅ No tests made less strict
  • ✅ No eslint-disable comments added
  • ✅ Small focused change (1 file, 14 lines total)

Phase 4: Code Quality ✅

IMPLEMENTATION ANALYSIS:

  • Consistency: Uses the same threshold pattern as other pipeline stages (lines 715, 805, 829)
  • Comments: Updated to be more descriptive about the ~13ms marshaling overhead
  • No Logic Bugs: Simple threshold comparison change with no edge cases
  • Performance: Change should reduce overhead for 4-5 file incremental builds
  • Dual Engine Compliance: Both native and JS paths still produce identical results

CODE LOCATIONS (src/domain/graph/builder/stages/build-edges.ts):

  • Lines 805: ctx.fileSymbols.size > ctx.config.build.smallFilesThreshold (import edges)
  • Lines 829: ctx.fileSymbols.size > ctx.config.build.smallFilesThreshold (call edges)

Phase 5: Backlog Compliance ✅

Zero-dep: ✓ — No new runtime dependencies added
Foundation-aligned: ✓ — Builds on existing tree-sitter/SQLite infrastructure, improves performance without violating any principles
Problem-fit: 4/5 — Improves performance for incremental builds, which directly helps AI agents by reducing round-trip times for common development workflows
Breaking: No — Purely internal optimization, no API or behavior changes visible to users
Tier: 1 (zero-dep ✓ + foundation-aligned ✓)


Root Cause Analysis

Problem Being Solved: napi-rs marshaling overhead exceeding computation savings for small incremental builds (4-5 files)
Why This Approach: Aligns threshold with existing config pattern used throughout codebase
Risk Assessment: Low risk - internal performance optimization with no behavioral changes

Backlog Compliance

  • Zero-dep: ✓ — no new dependencies
  • Foundation-aligned: ✓ — improves existing tree-sitter/SQLite pipeline
  • Problem-fit: 4 — reduces build times for incremental development workflows
  • Breaking: No — internal optimization only
  • Tier: 1

Critical Concerns

None. This is a well-executed performance optimization that follows established patterns.

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Solid performance optimization with clear measurable benefit (~13ms savings), follows codebase patterns, no risks identified, good documentation in comments and commit message.

Minor notes: The test plan mentions manual verification with --timing flag - consider running this before merge to validate the performance claim.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 16, 2026

Greptile Summary

This PR replaces two hardcoded > 3 guards in build-edges.ts (for both import-edge and call-edge native paths) with > ctx.config.build.smallFilesThreshold (default 5), using the same configurable threshold already applied in resolve-imports.ts, build-structure.ts, finalize.ts, and pipeline.ts. The boundary semantics are preserved (exclusive > comparison, so ≤5 files use JS, ≥6 use native), and the updated comments appropriately drop the now-stale hardcoded numbers.

Confidence Score: 5/5

Safe to merge — a clean, single-line threshold alignment with no logic changes or correctness risk.

The change replaces a hardcoded literal 3 with the already-established smallFilesThreshold constant (default 5), matching every other pipeline stage. Boundary semantics (exclusive >) are correct and consistent, the fallback to JS is still present, and both engines are expected to produce identical results per project policy.

No files require special attention.

Important Files Changed

Filename Overview
src/domain/graph/builder/stages/build-edges.ts Replaces hardcoded > 3 guard for native edge-building with > ctx.config.build.smallFilesThreshold (default 5), aligning this stage with the same configurable threshold used by resolve-imports, build-structure, finalize, and pipeline.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[buildEdges called] --> B{isFullBuild OR\nfileSymbols.size >\nsmallFilesThreshold?}
    B -- yes, native available --> C[useNativeImportEdges = true]
    B -- no / no native --> D[buildImportEdges JS]
    C --> E[buildImportEdgesNative]
    E --> F{0 edges but\nhas imports?}
    F -- yes --> G[fallback: buildImportEdges JS]
    F -- no --> H[call-edge decision]
    D --> H
    G --> H
    H --> I{isFullBuild OR\nfileSymbols.size >\nsmallFilesThreshold?}
    I -- yes, native available --> J[buildCallEdgesNative]
    I -- no / no native --> K[buildCallEdgesJS]
    J --> L[insert edges]
    K --> L
Loading

Reviews (1): Last reviewed commit: "perf(native): raise native edge-building..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

1 functions changed5 callers affected across 3 files

  • buildEdges in src/domain/graph/builder/stages/build-edges.ts:772 (5 transitive callers)

@carlos-alm carlos-alm merged commit dfa7424 into main Apr 16, 2026
17 checks passed
@carlos-alm carlos-alm deleted the perf/raise-native-edge-threshold branch April 16, 2026 02:56
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native: raise native edge-building threshold to avoid napi overhead on small incremental builds

1 participant