fix: native no-op rebuild regression (~80x slower than WASM)#360
fix: native no-op rebuild regression (~80x slower than WASM)#360carlos-alm merged 3 commits intomainfrom
Conversation
When the native engine provides pre-computed CFG data for all functions, `needsFallback` is false and `extToLang` was never built. Since native- parsed symbols lack `_langId`, the langId lookup in buildCFGData fell through to null and skipped every file — leaving cfg_blocks empty. On each subsequent incremental build the empty-table guard triggered a full re-parse of all files (~300 ms), making native no-op rebuilds ~80× slower than WASM (319 ms vs 4 ms). Move the extToLang construction before the needsFallback check so it is always available. Apply the same fix to buildDataflowEdges defensively. Impact: 2 functions changed, 13 affected
|
Claude finished @carlos-alm's task —— View job PR Review: Native No-op Rebuild Regression FixTodo List:
Root Cause AnalysisProblem Being Solved: Native engine builds were ~80x slower than WASM builds on no-op rebuilds (319ms vs 7-8ms). The issue was that Why This Approach: Moving Risk Assessment: LOW RISK - This is a defensive fix that ensures consistent behavior between native and WASM engines. The change is minimal and surgical. Critical ConcernsNone found. This is a well-targeted performance fix. Detailed AnalysisCode Changes ReviewThe fix is applied identically to both Before: // Always build ext→langId map so native-only builds (where _langId is unset)
// can still derive the language from the file extension.
const extToLang = new Map();
for (const entry of LANGUAGE_REGISTRY) {
for (const ext of entry.extensions) {
extToLang.set(ext, entry.id);
}
}Technical Correctness✅ Architecture preserved: Native data is still prioritized when available Test Evidence
Final Recommendation
|
Greptile SummaryThis PR fixes a performance regression where native-engine no-op rebuilds were ~80× slower than WASM by ensuring Key changes:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildCFGData / buildDataflowEdges called] --> B[Build extToLang map from LANGUAGE_REGISTRY]
B --> C{Scan fileSymbols\nfor needsFallback?}
C -- Yes --> D[Import parser.js\nCreate WASM parsers]
C -- No --> E[parsers = null\ngetParserFn = null]
D --> F[getParserFn = mod.getParser]
E --> G[Process each file]
F --> G
G --> H{symbols._tree exists\nor allNative?}
H -- No tree & not allNative --> I{getParserFn set?}
I -- No --> J[skip file]
I -- Yes --> K[extToLang.get ext\nWASM parse tree]
H -- Has tree or allNative --> L{langId set?}
K --> L
L -- No --> M[langId = extToLang.get ext]
L -- Yes --> N[Process CFG / Dataflow]
M --> N
N --> O[Insert rows into DB]
style B fill:#d4edda,stroke:#28a745
style J fill:#f8d7da,stroke:#dc3545
Last reviewed commit: c34fe2a |
|
Claude finished @carlos-alm's task —— View job PR Review: Native No-op Rebuild Regression FixTodo List:
Root Cause AnalysisProblem Being Solved: Native engine builds were ~80x slower than WASM builds on no-op rebuilds (319ms vs 7-8ms). The issue was that Why This Approach: Moving Risk Assessment: LOW RISK - This is a defensive fix that ensures consistent behavior between native and WASM engines. The change is minimal and surgical. Critical ConcernsNone found. This is a well-targeted performance fix. Detailed AnalysisCode Changes ReviewThe fix is applied identically to both Before: // Always build ext→langId map so native-only builds (where _langId is unset)
// can still derive the language from the file extension.
const extToLang = new Map();
for (const entry of LANGUAGE_REGISTRY) {
for (const ext of entry.extensions) {
extToLang.set(ext, entry.id);
}
}Technical Correctness✅ Architecture preserved: Native data is still prioritized when available Code Quality Notes
These don't affect functionality but represent minor tech debt. Test Evidence
Final Recommendation
|
Clean up four stale null-checks identified by Greptile review: - cfg.js:1120 and dataflow.js:1074: drop `!extToLang ||` from guards - cfg.js:1143 and dataflow.js:1097: simplify ternary to direct .get() Impact: 2 functions changed, 0 affected
|
All 4 dead-code guards identified by @greptileai cleaned up in c34fe2a:
|
|
Claude finished @carlos-alm's task —— View job PR Review: Native No-op Rebuild Regression FixTodo List:
Root Cause AnalysisProblem Being Solved: Native engine builds were ~80x slower than WASM builds on no-op rebuilds (319ms vs 7-8ms). The issue was that Why This Approach: Moving Risk Assessment: LOW RISK - This is a defensive fix that ensures consistent behavior between native and WASM engines. The change is minimal and surgical. Critical ConcernsNone found. This is a well-targeted performance fix. Detailed AnalysisCode Changes ReviewThe fix is applied identically to both Before: // Always build ext→langId map so native-only builds (where _langId is unset)
// can still derive the language from the file extension.
const extToLang = new Map();
for (const entry of LANGUAGE_REGISTRY) {
for (const ext of entry.extensions) {
extToLang.set(ext, entry.id);
}
}Technical Correctness✅ Architecture preserved: Native data is still prioritized when available Test Evidence
Final Recommendation
|
1 participant
Summary
buildCFGDataincfg.jsonly built theextToLangmap when WASM fallback was needed. When the native engine (3.0.4) provides pre-computed CFG for all functions,extToLangstayednull. Since native-parsed symbols lack_langId, thelangIdlookup fell through tonulland every file was skipped — leavingcfg_blocksempty.builder.js:534triggered a fullparseFilesAutore-parse of all files (~300ms), making native no-op rebuilds ~80× slower than WASM.extToLangconstruction before theneedsFallbackcheck so it is always available. Same defensive fix applied tobuildDataflowEdges.Test plan
cfg_blocksis populated (13816 rows) after native full build