From 01e49a12bfdaa9a03f2cccec2d1b352ce9248648 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 21 May 2026 17:01:03 +0200 Subject: [PATCH] perf(registry): per-file full-result cache for cbm_registry_resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit THE big win. Pinpoint-instrumented profiling showed 98.7% of resolve_calls CPU on kubernetes (881s out of 893s) lived inside the 'resolve' sub-step — cbm_pipeline_lsp_target_node when LSP hits, cbm_registry_resolve strategy chain when it misses. The LSP path is fast; the strategy chain isn't. cbm_registry_resolve's strategy chain (import_map -> same_module -> unique_name -> suffix_match+scoring) is deterministic for a given (callee_name, module_qn) pair. module_qn is CONSTANT within a file, so within one file the same callee_name resolves identically every time. K8s files have ~200 calls but only ~50 unique callee_names — so ~75% of strategy-chain walks are redundant. Added a TLS hash cache (_resolve_cache, registry.c) keyed by callee_name with value cbm_resolution_t. cbm_registry_resolve checks the cache first; on miss it runs the full chain and caches BOTH positive and negative results (caching the negative is just as valuable — same name asks the same question). API in pipeline.h: void cbm_registry_resolve_cache_begin(int estimated_capacity); void cbm_registry_resolve_cache_end(void); Two smaller chunks landed in the same commit: - src/pipeline/registry.c: per-file import-map prefix -> module-qn hash (cbm_registry_import_map_cache_*). resolve_import_map's linear strcmp scan becomes O(1). Flat impact in this run because the LSP path bypasses cbm_registry_resolve for most K8s calls, but useful for projects without strong LSP coverage. - src/pipeline/pass_parallel.c: reuse lsp_target as target_node when try_field_type_hint didn't override the QN. Saves 1 cbm_gbuf_find_by_qn lookup per LSP-resolved call. Also added 6 sub-counters inside resolve_file_calls (time_ns_rc_*) logged as parallel.resolve.calls_breakdown / breakdown2 to keep durable telemetry on which sub-step dominates. Benchmark on kubernetes (12 workers, 20,650 files): - parallel_resolve wall: 90,531 -> 36,376 ms (-59.8% vs C2; -64.8% vs original 103,281 baseline) - resolve_calls CPU: 893,657 -> 277,556 ms (-68.9%) - 'resolve' sub-counter: 881,650 -> 266,521 ms (-69.8%) ← target hit - resolve_usages CPU bonus: 10,986 -> 5,594 ms (-49.1%, secondary effect — fewer ambiguous resolutions to scrub) - Total pipeline.done: 115,918 -> 61,485 ms (-47%) - User CPU total: 953.7s -> 375.1s (-60.7%) - Peak RSS: +430 MB for cached cbm_resolution_t structs (~12 live caches at a time) - Tests: 3616/0 - Graph: nodes 297,082 EXACT match, edges +/- 0.2% - zod/serilog sanity: nodes exact, edges within tolerance Effective hit rate: predicted 75%, measured ~70%. The ~30% true cache misses (unique callee_names per file) still run the full chain — next-tier optimizations: widen scope to per-worker so common names like 'Get'/'Add' are computed once per worker not once per file, or pre-filter candidate lists by import reachability at file entry to shrink the chain's inner cost. Cumulative trajectory from original broken state: ~35 min K8s cold index -> 63.6s wall = ~33x total speedup. --- src/pipeline/pass_parallel.c | 79 ++++++++++++++++++++- src/pipeline/pipeline.h | 16 +++++ src/pipeline/registry.c | 134 +++++++++++++++++++++++++++++++---- 3 files changed, 213 insertions(+), 16 deletions(-) diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 9b2d3ce30..136fc30a3 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -925,6 +925,15 @@ typedef struct { * cbm_parallel_for synchronization barrier at the end. */ _Atomic uint64_t time_ns_total_loop; _Atomic int total_files_visited; + /* Sub-breakdowns inside resolve_file_calls — finds the 553µs-per- + * iteration hot path that the high-level resolve_calls counter + * doesn't pinpoint. */ + _Atomic uint64_t time_ns_rc_lsp_lookup; /* lsp_idx + fallback scan */ + _Atomic uint64_t time_ns_rc_resolve; /* lsp_target_node OR registry_resolve */ + _Atomic uint64_t time_ns_rc_hint; /* try_field_type_hint */ + _Atomic uint64_t time_ns_rc_target; /* gbuf_find_by_qn for target */ + _Atomic uint64_t time_ns_rc_emit; /* emit_service_edge */ + _Atomic uint64_t time_ns_rc_source; /* find_source_node */ } resolve_ctx_t; /* Minimum buffer space needed per arg JSON object */ @@ -1592,8 +1601,10 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB if (!call->callee_name) { continue; } + uint64_t _rc_t0 = extract_now_ns(); const cbm_gbuf_node_t *source_node = find_source_node(rc->main_gbuf, rc->project_name, rel, call->enclosing_func_qn); + atomic_fetch_add_explicit(&rc->time_ns_rc_source, extract_now_ns() - _rc_t0, memory_order_relaxed); if (!source_node) { continue; } @@ -1605,6 +1616,7 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB * depending on whether parallel mode kicked in. */ cbm_resolution_t res = {0}; const CBMResolvedCall *lsp = NULL; + _rc_t0 = extract_now_ns(); if (lsp_idx && call->enclosing_func_qn) { char key[1024]; int kn = snprintf(key, sizeof(key), "%s|%s", @@ -1619,6 +1631,9 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB * name). Keeps semantics identical. */ lsp = cbm_pipeline_find_lsp_resolution(&result->resolved_calls, call); } + atomic_fetch_add_explicit(&rc->time_ns_rc_lsp_lookup, extract_now_ns() - _rc_t0, memory_order_relaxed); + _rc_t0 = extract_now_ns(); + const cbm_gbuf_node_t *lsp_target = NULL; if (lsp) { /* Canonicalise to the gbuf node's QN so res.qualified_name matches * the gbuf even when the cross-file fallback had to prefix the @@ -1626,7 +1641,7 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB * empty — the LSP was confident but its target isn't in the gbuf * (external/unindexed), so drop the edge rather than fall back to * the registry resolver, matching prior single-lookup semantics. */ - const cbm_gbuf_node_t *lsp_target = + lsp_target = cbm_pipeline_lsp_target_node(rc->main_gbuf, rc->project_name, lsp->callee_qn); if (lsp_target) { res.qualified_name = lsp_target->qualified_name; @@ -1639,8 +1654,11 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB res = cbm_registry_resolve(rc->registry, call->callee_name, module_qn, imp_keys, imp_vals, imp_count); } + atomic_fetch_add_explicit(&rc->time_ns_rc_resolve, extract_now_ns() - _rc_t0, memory_order_relaxed); + _rc_t0 = extract_now_ns(); try_field_type_hint(rc, &res, call->callee_name, source_node->id); + atomic_fetch_add_explicit(&rc->time_ns_rc_hint, extract_now_ns() - _rc_t0, memory_order_relaxed); if (!res.qualified_name || res.qualified_name[0] == '\0') { if (cbm_service_pattern_route_method(call->callee_name) != NULL) { @@ -1653,12 +1671,25 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB } continue; } - const cbm_gbuf_node_t *target_node = cbm_gbuf_find_by_qn(rc->main_gbuf, res.qualified_name); + /* Reuse lsp_target as target_node when LSP resolved — avoids a + * second cbm_gbuf_find_by_qn lookup. try_field_type_hint may have + * upgraded res.qualified_name to a different candidate, in which + * case we must re-resolve. */ + _rc_t0 = extract_now_ns(); + const cbm_gbuf_node_t *target_node; + if (lsp_target && res.qualified_name == lsp_target->qualified_name) { + target_node = lsp_target; + } else { + target_node = cbm_gbuf_find_by_qn(rc->main_gbuf, res.qualified_name); + } + atomic_fetch_add_explicit(&rc->time_ns_rc_target, extract_now_ns() - _rc_t0, memory_order_relaxed); if (!target_node || source_node->id == target_node->id) { continue; } + _rc_t0 = extract_now_ns(); emit_service_edge(ws->local_edge_buf, source_node, target_node, call, &res, module_qn, rc->registry, rc->main_gbuf, imp_keys, imp_vals, imp_count); + atomic_fetch_add_explicit(&rc->time_ns_rc_emit, extract_now_ns() - _rc_t0, memory_order_relaxed); ws->calls_resolved++; } if (lsp_idx) { @@ -1946,6 +1977,20 @@ static void resolve_worker(int worker_id, void *ctx_ptr) { * cache is sound; invalidated at file exit. */ cbm_registry_reach_cache_begin(result->calls.count + result->usages.count + 64); + /* Per-file import-map prefix → module-qn hash. resolve_import_map + * was doing O(imports) linear strcmp per call; with this it + * becomes O(1). Keys/values borrowed from imp_keys/imp_vals + * which outlive this scope. */ + cbm_registry_import_map_cache_begin(imp_keys, imp_vals, imp_count); + + /* THE BIG ONE: per-file cache of cbm_registry_resolve results. + * Same callee_name in multiple call sites resolves identically + * within a file (module_qn is fixed) — first call walks the + * strategy chain, repeats are O(1). On K8s this targets the + * 98.7% hot spot in resolve_file_calls (881 of 893s CPU). */ + cbm_registry_resolve_cache_begin( + result->calls.count + result->usages.count + 64); + char *module_qn = cbm_pipeline_fqn_module(rc->project_name, rel); /* ── Cross-file LSP (FUSED) ───────────────────────────── @@ -2092,6 +2137,8 @@ static void resolve_worker(int worker_id, void *ctx_ptr) { atomic_fetch_add_explicit(&rc->time_ns_semantic, extract_now_ns() - _ph_t0, memory_order_relaxed); cbm_registry_reach_cache_end(); + cbm_registry_import_map_cache_end(); + cbm_registry_resolve_cache_end(); free(module_qn); free_import_map(imp_keys, imp_vals, imp_count); @@ -2247,5 +2294,33 @@ int cbm_parallel_resolve(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t *files, "resolve_throws", thr_buf, "resolve_rw", rw_buf, "resolve_semantic", sem_buf); + + char src_buf[32], lsp_buf[32], rsv_buf[32], hnt_buf[32], tgt_buf[32], emt_buf[32]; + snprintf(src_buf, sizeof(src_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_source, + memory_order_relaxed) / 1000000ULL)); + snprintf(lsp_buf, sizeof(lsp_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_lsp_lookup, + memory_order_relaxed) / 1000000ULL)); + snprintf(rsv_buf, sizeof(rsv_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_resolve, + memory_order_relaxed) / 1000000ULL)); + snprintf(hnt_buf, sizeof(hnt_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_hint, + memory_order_relaxed) / 1000000ULL)); + snprintf(tgt_buf, sizeof(tgt_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_target, + memory_order_relaxed) / 1000000ULL)); + snprintf(emt_buf, sizeof(emt_buf), "%llu", + (unsigned long long)(atomic_load_explicit(&rc.time_ns_rc_emit, + memory_order_relaxed) / 1000000ULL)); + cbm_log_info("parallel.resolve.calls_breakdown", + "find_source", src_buf, + "lsp_lookup", lsp_buf, + "resolve", rsv_buf); + cbm_log_info("parallel.resolve.calls_breakdown2", + "field_hint", hnt_buf, + "find_target", tgt_buf, + "emit_edge", emt_buf); return 0; } diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index 5e293bad0..4d2a0e1c6 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -139,6 +139,22 @@ cbm_resolution_t cbm_registry_resolve(const cbm_registry_t *r, const char *calle void cbm_registry_reach_cache_begin(int estimated_capacity); void cbm_registry_reach_cache_end(void); +/* Per-file import-map prefix → module-QN hash. Turns the linear + * strcmp scan inside resolve_import_map into O(1). Keys/values are + * BORROWED — caller must keep the import_map arrays alive for the + * cache lifetime. Invalidate between files via _end. */ +void cbm_registry_import_map_cache_begin(const char **keys, const char **vals, int count); +void cbm_registry_import_map_cache_end(void); + +/* Per-file full-result cache for cbm_registry_resolve. The same + * callee_name appears in many call sites within a file; module_qn + * is constant per file so each name resolves identically. First + * lookup does the full strategy chain; repeats are O(1) hash hits. + * This eliminates ~75% of the resolve-chain work on K8s where the + * same names ("Get", "Add", "New", etc) appear hundreds of times. */ +void cbm_registry_resolve_cache_begin(int estimated_capacity); +void cbm_registry_resolve_cache_end(void); + /* Check if a qualified name exists in the registry. */ bool cbm_registry_exists(const cbm_registry_t *r, const char *qn); diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index 37ea24d52..537bac443 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -193,6 +193,84 @@ void cbm_registry_reach_cache_end(void) { _reach_cache = NULL; } +/* ── Per-file import-map prefix → module-QN hash ────────────────── + * + * resolve_import_map does a linear strcmp scan over the per-file + * import list for every call (and every usage, throw, rw, def). On + * kubernetes typical files have 20-30 imports and ~200 calls. That's + * 4000-6000 strcmps per file × 20K files = ~80M strcmps. + * + * Build a hash from prefix → module_qn ONCE per file at file entry. + * Each lookup becomes O(1). Keys and values are BORROWED from the + * caller's import_map — they outlive the cache scope. */ +static CBM_TLS CBMHashTable *_import_map_cache = NULL; + +void cbm_registry_import_map_cache_begin(const char **keys, const char **vals, int count) { + if (_import_map_cache) { + cbm_ht_free(_import_map_cache); + _import_map_cache = NULL; + } + if (!keys || !vals || count <= 0) return; + _import_map_cache = cbm_ht_create((uint32_t)count * 2u + 8u); + if (!_import_map_cache) return; + for (int i = 0; i < count; i++) { + if (keys[i] && vals[i]) { + cbm_ht_set(_import_map_cache, keys[i], (void *)(uintptr_t)vals[i]); + } + } +} + +void cbm_registry_import_map_cache_end(void) { + if (!_import_map_cache) return; + /* Keys/values borrowed from caller — no free callback needed. */ + cbm_ht_free(_import_map_cache); + _import_map_cache = NULL; +} + +/* ── Per-file full-result cache for cbm_registry_resolve ────────── + * + * THE big one: 98.7% of resolve_calls CPU on kubernetes lives inside + * cbm_registry_resolve's strategy chain (suffix_match iterating + * hundreds of candidates per common name like "Get"/"Add"/"New" + * × 1M LSP-miss calls = ~800s of work). + * + * Since module_qn is CONSTANT per file, the resolution for a given + * callee_name within a file is also constant — same name resolves + * to the same QN every time. Cache the full cbm_resolution_t result + * keyed by callee_name; first lookup does the full chain, repeats + * are O(1). On K8s a typical file has ~200 calls but only ~50 + * unique callee_names → ~75% hit rate → ~75% of the resolve cost + * eliminated. */ +typedef struct { + cbm_resolution_t res; +} resolve_cache_entry_t; + +static CBM_TLS CBMHashTable *_resolve_cache = NULL; +/* Entries are malloc'd; keys are strdup'd. Both freed in _end. */ + +static void resolve_cache_free_entry(const char *key, void *val, void *ud) { + (void)ud; + free((char *)key); + free(val); +} + +void cbm_registry_resolve_cache_begin(int estimated_capacity) { + if (_resolve_cache) { + cbm_ht_foreach(_resolve_cache, resolve_cache_free_entry, NULL); + cbm_ht_free(_resolve_cache); + _resolve_cache = NULL; + } + if (estimated_capacity < 32) estimated_capacity = 32; + _resolve_cache = cbm_ht_create((uint32_t)estimated_capacity); +} + +void cbm_registry_resolve_cache_end(void) { + if (!_resolve_cache) return; + cbm_ht_foreach(_resolve_cache, resolve_cache_free_entry, NULL); + cbm_ht_free(_resolve_cache); + _resolve_cache = NULL; +} + /* Check if candidate's module prefix appears in import map values. * Uses stack buffer to avoid malloc/free per call in hot resolution loop. * Per-file memoization via TLS cache: repeated lookups of the same @@ -388,12 +466,17 @@ static cbm_resolution_t resolve_import_map(const cbm_registry_t *r, const char * return empty_result(); } - /* Find prefix in import map keys */ + /* Find prefix in import map keys. Prefer the per-file TLS hash + * cache (O(1)) over the linear scan when available. */ const char *resolved = NULL; - for (int i = 0; i < map_count; i++) { - if (strcmp(keys[i], prefix) == 0) { - resolved = vals[i]; - break; + if (_import_map_cache) { + resolved = (const char *)cbm_ht_get(_import_map_cache, prefix); + } else { + for (int i = 0; i < map_count; i++) { + if (strcmp(keys[i], prefix) == 0) { + resolved = vals[i]; + break; + } } } if (!resolved) { @@ -530,6 +613,17 @@ cbm_resolution_t cbm_registry_resolve(const cbm_registry_t *r, const char *calle return empty_result(); } + /* Per-file cache: same callee_name in N call sites → 1 chain walk + * + N-1 O(1) hash hits. module_qn is constant per file so the + * cache key only needs callee_name. */ + if (_resolve_cache) { + resolve_cache_entry_t *cached = + (resolve_cache_entry_t *)cbm_ht_get(_resolve_cache, callee_name); + if (cached) { + return cached->res; + } + } + /* Split callee: "pkg.Func" → prefix="pkg", suffix="Func" */ char prefix[CBM_SZ_256] = {0}; const char *suffix = NULL; @@ -549,18 +643,30 @@ cbm_resolution_t cbm_registry_resolve(const cbm_registry_t *r, const char *calle /* Strategy 1: import map */ cbm_resolution_t res = resolve_import_map(r, prefix, suffix, import_map_keys, import_map_vals, import_map_count); - if (res.qualified_name && res.qualified_name[0]) { - return res; + if (!(res.qualified_name && res.qualified_name[0])) { + /* Strategy 2: same module */ + res = resolve_same_module(r, callee_name, suffix, module_qn); + } + if (!(res.qualified_name && res.qualified_name[0])) { + /* Strategy 3+4: name lookup */ + res = resolve_name_lookup(r, callee_name, module_qn, import_map_vals, import_map_count); } - /* Strategy 2: same module */ - res = resolve_same_module(r, callee_name, suffix, module_qn); - if (res.qualified_name && res.qualified_name[0]) { - return res; + /* Cache the result (including empty — caching the negative answer + * is just as valuable; same name asks the same question). */ + if (_resolve_cache) { + resolve_cache_entry_t *e = (resolve_cache_entry_t *)malloc(sizeof(*e)); + if (e) { + e->res = res; + char *kdup = strdup(callee_name); + if (kdup) { + cbm_ht_set(_resolve_cache, kdup, e); + } else { + free(e); + } + } } - - /* Strategy 3+4: name lookup */ - return resolve_name_lookup(r, callee_name, module_qn, import_map_vals, import_map_count); + return res; } /* ── Fuzzy Resolve ──────────────────────────────────────────────── */