diff --git a/.alfonso/release-notes/next/views-store-compact-payloads.md b/.alfonso/release-notes/next/views-store-compact-payloads.md new file mode 100644 index 000000000..268e6ea33 --- /dev/null +++ b/.alfonso/release-notes/next/views-store-compact-payloads.md @@ -0,0 +1 @@ +- With views on, each call-graph database is about 45% smaller (1.97 GB to 1.05 GB for AFT's own repository) and builds about 10% faster, with identical answers. diff --git a/Cargo.lock b/Cargo.lock index c00e13331..f75219772 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -69,6 +69,7 @@ dependencies = [ "dekobon-tree-sitter-groovy", "env_logger", "filetime", + "flate2", "fsevent-sys", "futures-util", "getrandom 0.3.4", diff --git a/benchmarks/aft-search/slice-descriptors/land-views-store-compact-payloads.json b/benchmarks/aft-search/slice-descriptors/land-views-store-compact-payloads.json new file mode 100644 index 000000000..0660584a6 --- /dev/null +++ b/benchmarks/aft-search/slice-descriptors/land-views-store-compact-payloads.json @@ -0,0 +1,8 @@ +{ + "slice_class": "non_ranking", + "targeted_mechanism": "none", + "kind": "harness", + "fixtures": [ + "harness-goldens" + ] +} diff --git a/crates/aft/Cargo.toml b/crates/aft/Cargo.toml index 075fa772e..ca524bd90 100644 --- a/crates/aft/Cargo.toml +++ b/crates/aft/Cargo.toml @@ -203,6 +203,10 @@ tokio-rustls = { version = "0.26", default-features = false, features = ["ring", tokio-util = { version = "0.7", features = ["io"] } futures-util = "0.3" async-compression = { version = "0.4", features = ["tokio", "gzip", "zlib", "brotli"] } +# Deflate for the call-graph view's per-file JSON payloads. Already in the +# build through async-compression, image and ureq; listed here to call it +# synchronously. +flate2 = "1.1" rcgen = "0.13" # Shared CortexKit-neutral path-canonicalization primitive (ProjectRootId). # Lives in github.com/cortexkit/commons (product-neutral, used without subc); diff --git a/crates/aft/src/callgraph_store/mod.rs b/crates/aft/src/callgraph_store/mod.rs index 2ebe33de0..69f74b1b5 100644 --- a/crates/aft/src/callgraph_store/mod.rs +++ b/crates/aft/src/callgraph_store/mod.rs @@ -13406,7 +13406,26 @@ pub(crate) fn initialize_schema(conn: &Connection) -> Result<()> { CREATE TABLE IF NOT EXISTS staging_ref_context ( ref_id TEXT PRIMARY KEY, caller_symbol TEXT - );", + ); + + -- Node ids hold 64 bits of a digest of file, kind and scoped name + -- (`node_id`). Two different symbols with the same id fail the write + -- instead of one silently replacing the other. + CREATE TRIGGER IF NOT EXISTS nodes_id_collision_on_insert BEFORE INSERT ON nodes + WHEN EXISTS ( + SELECT 1 FROM nodes WHERE id = NEW.id AND ( + file_path IS NOT NEW.file_path + OR scoped_name IS NOT NEW.scoped_name + OR kind IS NOT NEW.kind + ) + ) + BEGIN SELECT RAISE(ABORT, 'call-graph node id collision'); END; + CREATE TRIGGER IF NOT EXISTS nodes_id_collision_on_update + BEFORE UPDATE OF file_path, scoped_name, kind ON nodes + WHEN OLD.file_path IS NOT NEW.file_path + OR OLD.scoped_name IS NOT NEW.scoped_name + OR OLD.kind IS NOT NEW.kind + BEGIN SELECT RAISE(ABORT, 'call-graph node id collision'); END;", )?; // CREATE TABLE IF NOT EXISTS leaves older tables untouched. Upgrade the // column before creating indexes that reference it; readiness still forces @@ -13684,7 +13703,10 @@ fn ensure_database_ready(conn: &Connection) -> Result<()> { /// `super()`, TypeScript object and factory members, and Rust trait and /// associated calls resolve to their real targets; Go, Python and TypeScript /// no longer guess callers by method name alone. -const BUILD_OUTPUT_VERSION: &str = "v23-resolver-precision"; +/// - v24: node ids carry 64 bits of their digest instead of 256, views stores +/// spell them `vp`/`vt` plus the digest, and two indexes no statement used +/// (edges by target node, dispatch hints by method name) are gone. +const BUILD_OUTPUT_VERSION: &str = "v24-short-node-ids"; const H_HEADER_LANGUAGE_META_KEY: &str = "h_header_language_revision"; @@ -13694,7 +13716,7 @@ mod h_header_invalidation_tests { #[test] fn h_header_legacy_migration_is_targeted_atomic_and_once() { - assert_eq!(BUILD_OUTPUT_VERSION, "v23-resolver-precision"); + assert_eq!(BUILD_OUTPUT_VERSION, "v24-short-node-ids"); let temp = tempfile::tempdir().unwrap(); std::fs::create_dir_all(temp.path().join("root")).unwrap(); // Parse counters key files by the canonical path the store reads them @@ -14206,13 +14228,11 @@ const SECONDARY_INDEXES: &[(&str, &str)] = &[ "file_dependencies(dep_file)", ), ("idx_edges_source_kind", "edges(source_node, kind)"), - ("idx_edges_target_kind", "edges(target_node, kind)"), ( "idx_edges_target_file_symbol", "edges(target_file, target_symbol, kind)", ), ("idx_edges_ref_id", "edges(ref_id, kind)"), - ("idx_dispatch_hints_method", "dispatch_hints(method_name)"), ("idx_dispatch_hints_file", "dispatch_hints(file)"), ( "idx_backend_file_state_file", @@ -14224,10 +14244,20 @@ const SECONDARY_INDEXES: &[(&str, &str)] = &[ ), ]; +/// Indexes older builds created that no statement uses any more: no query +/// filters edges by target node or dispatch hints by method name. Dropping +/// them keeps a store opened by this build from carrying their pages. +const RETIRED_SECONDARY_INDEXES: &[&str] = &["idx_edges_target_kind", "idx_dispatch_hints_method"]; + fn create_secondary_indexes_sql() -> String { - SECONDARY_INDEXES + RETIRED_SECONDARY_INDEXES .iter() - .map(|(name, target)| format!("CREATE INDEX IF NOT EXISTS {name} ON {target};")) + .map(|name| format!("DROP INDEX IF EXISTS {name};")) + .chain( + SECONDARY_INDEXES + .iter() + .map(|(name, target)| format!("CREATE INDEX IF NOT EXISTS {name} ON {target};")), + ) .collect::>() .join("\n") } @@ -15993,9 +16023,18 @@ fn selection_range(source: &str, scoped_name: &str, name: &str, fallback: &Range } } +/// A node's id: its class prefix and the first 64 bits of a BLAKE3 digest of +/// its file, kind and scoped name, as 16 hex digits. +/// +/// Node ids are repeated in every ref, edge and index that names a caller or a +/// target, so their length is a large share of the store. 64 bits keep them +/// unique in practice; the store's insert and update triggers refuse two +/// different symbols with the same id (`initialize_schema`) rather than merging +/// them. Hex digits of a prefix sort as the full digest does, so ordering by id +/// is unchanged. fn node_id(rel_path: &str, kind: &str, scoped_name: &str) -> String { if scoped_name == TOP_LEVEL_SYMBOL { - return format!("top:{}", hash_to_hex(blake3::hash(rel_path.as_bytes()))); + return format!("top:{}", short_hash_hex(blake3::hash(rel_path.as_bytes()))); } // Length-delimited fields avoid aliasing paths or names containing colons. let mut hash = blake3::Hasher::new(); @@ -16003,11 +16042,23 @@ fn node_id(rel_path: &str, kind: &str, scoped_name: &str) -> String { hash.update(&(field.len() as u64).to_le_bytes()); hash.update(field.as_bytes()); } - format!("pos:symbol:{}", hash_to_hex(hash.finalize())) + format!("pos:symbol:{}", short_hash_hex(hash.finalize())) } +/// A views store's node id: `vp` plus 16 hex digits for a symbol, `vt` plus +/// 16 hex digits for a file's top level. +/// +/// The digests are `node_id`'s. Views stores never hold `node_id`'s longer +/// spellings, so the short class letters only have to keep the two classes +/// apart and in the same order: symbol ids still sort before top-level ids, and +/// ids of one class still sort by digest. pub(crate) fn view_node_id(rel_path: &str, kind: &str, scoped_name: &str) -> String { - format!("view:symbol:{}", node_id(rel_path, kind, scoped_name)) + let id = node_id(rel_path, kind, scoped_name); + match (id.strip_prefix("pos:symbol:"), id.strip_prefix("top:")) { + (Some(digest), _) => format!("vp{digest}"), + (_, Some(digest)) => format!("vt{digest}"), + _ => unreachable!("node_id returns a symbol or top-level id"), + } } fn build_call_refs( @@ -26182,6 +26233,11 @@ fn hash_to_hex(hash: blake3::Hash) -> String { hash.to_hex().to_string() } +/// The first 64 bits of a digest as 16 hex digits. +fn short_hash_hex(hash: blake3::Hash) -> String { + hash.to_hex()[..16].to_string() +} + fn hash_from_hex(value: &str) -> Option { let bytes = hex_to_bytes(value)?; Some(blake3::Hash::from_bytes(bytes)) diff --git a/crates/aft/src/callgraph_store/position_identity_tests.rs b/crates/aft/src/callgraph_store/position_identity_tests.rs index 79bc98dce..a24710fbb 100644 --- a/crates/aft/src/callgraph_store/position_identity_tests.rs +++ b/crates/aft/src/callgraph_store/position_identity_tests.rs @@ -189,6 +189,66 @@ fn s4_two_scoped_names_share_a_range_without_identity_collision() { assert_ne!(nodes[0].id, nodes[1].id); } +#[test] +fn node_ids_are_short_digests_that_sort_like_the_full_digest() { + let id = node_id("a.rs", "function", "f"); + let hex = id.strip_prefix("pos:symbol:").unwrap(); + assert_eq!(hex.len(), 16); + assert!(hex.bytes().all(|byte| byte.is_ascii_hexdigit())); + let top = node_id("a.rs", "module", TOP_LEVEL_SYMBOL); + assert_eq!(top.strip_prefix("top:").unwrap().len(), 16); + // Ordering by the stored id must match ordering by the full digest it was + // cut from, since replies order nodes by id. + let mut ids = (0..200) + .map(|n| { + let mut hash = blake3::Hasher::new(); + hash.update(n.to_string().as_bytes()); + let full = hash.finalize(); + (short_hash_hex(full), hash_to_hex(full)) + }) + .collect::>(); + ids.sort_by(|left, right| left.0.cmp(&right.0)); + assert!(ids.windows(2).all(|pair| pair[0].1 < pair[1].1)); +} + +#[test] +fn two_symbols_with_one_node_id_fail_the_write_instead_of_merging() { + let conn = Connection::open_in_memory().unwrap(); + initialize_schema(&conn).unwrap(); + let insert = |scoped_name: &str| { + conn.execute( + "INSERT INTO nodes(id, file_path, name, scoped_name, kind, start_line, start_col, + end_line, end_col, range_ordinal, signature, exported, is_default_export, + is_type_like, is_callgraph_entry_point, provenance, is_test) + VALUES('pos:symbol:0123456789abcdef', 'a.rs', ?1, ?1, 'function', 1, 0, 1, 1, 0, + NULL, 0, 0, 0, 0, 'treesitter', 0) + ON CONFLICT(id) DO UPDATE SET name=excluded.name, scoped_name=excluded.scoped_name", + [scoped_name], + ) + }; + insert("f").unwrap(); + // The same symbol again is an ordinary upsert. + insert("f").unwrap(); + let error = insert("g").unwrap_err().to_string(); + assert!(error.contains("call-graph node id collision"), "{error}"); + let error = conn + .execute( + "INSERT OR REPLACE INTO nodes(id, file_path, name, scoped_name, kind, start_line, + start_col, end_line, end_col, range_ordinal, signature, exported, + is_default_export, is_type_like, is_callgraph_entry_point, provenance, is_test) + VALUES('pos:symbol:0123456789abcdef', 'b.rs', 'f', 'f', 'function', 9, 0, 9, 1, 0, + NULL, 0, 0, 0, 0, 'treesitter', 0)", + [], + ) + .unwrap_err() + .to_string(); + assert!(error.contains("call-graph node id collision"), "{error}"); + let scoped: String = conn + .query_row("SELECT scoped_name FROM nodes", [], |row| row.get(0)) + .unwrap(); + assert_eq!(scoped, "f"); +} + #[test] fn s4_node_identity_uses_path_kind_and_scoped_name_only() { assert_ne!( @@ -307,7 +367,7 @@ pub(crate) fn assert_current_graph(conn: &Connection) { ("edges", "target_node"), ("dispatch_hints", "caller_node"), ] { - let old: usize = conn.query_row(&format!("SELECT count(*) FROM {table} WHERE {column} IS NOT NULL AND {column} NOT LIKE 'top:%' AND {column} NOT LIKE 'pos:symbol:%' AND {column} NOT LIKE 'view:symbol:%'"), [], |r| r.get(0)).unwrap(); + let old: usize = conn.query_row(&format!("SELECT count(*) FROM {table} WHERE {column} IS NOT NULL AND {column} NOT LIKE 'top:%' AND {column} NOT LIKE 'pos:symbol:%' AND {column} NOT LIKE 'vp%' AND {column} NOT LIKE 'vt%'"), [], |r| r.get(0)).unwrap(); assert_eq!(old, 0, "old identity in {table}.{column}"); } } @@ -409,7 +469,7 @@ pub(crate) fn with_pre_s4_project(root: &Path, run: impl FnOnce() -> T) -> T pub(crate) fn seed_old_dispatch_hint(conn: &Connection) { conn.execute("INSERT INTO dispatch_hints SELECT 'pre-s4-hint', 'unique', id, file_path, start_line, 0, 0, 'treesitter' FROM nodes ORDER BY id LIMIT 1", []).unwrap(); - assert!(conn.query_row("SELECT count(*) FROM dispatch_hints WHERE caller_node NOT LIKE 'pos:symbol:%' AND caller_node NOT LIKE 'view:symbol:%'", [], |r| r.get::<_,usize>(0)).unwrap()>0); + assert!(conn.query_row("SELECT count(*) FROM dispatch_hints WHERE caller_node NOT LIKE 'pos:symbol:%' AND caller_node NOT LIKE 'vp%' AND caller_node NOT LIKE 'vt%'", [], |r| r.get::<_,usize>(0)).unwrap()>0); } #[test] diff --git a/crates/aft/src/views/materialization.rs b/crates/aft/src/views/materialization.rs index f4539f784..53b7711cd 100644 --- a/crates/aft/src/views/materialization.rs +++ b/crates/aft/src/views/materialization.rs @@ -131,6 +131,7 @@ pub(crate) fn apply_manifest_diff_profiled( mod module_digest; #[cfg(test)] pub(crate) mod parity; +pub(crate) mod payload; pub(crate) mod profile; mod resolution_facts; mod stable_rows; @@ -139,7 +140,7 @@ mod stable_rows; // per-reference skip optimization when reference ordinals collide, or Go // bindings whose package file listings were not recorded (an added sibling // `.go` file would never reselect them). -pub(crate) const MATERIALIZATION_VERSION: &str = "12"; +pub(crate) const MATERIALIZATION_VERSION: &str = "13"; const BASE_FINGERPRINT_MISMATCH: &str = "derived manifest fingerprint mismatch; cold materialization required"; @@ -432,11 +433,11 @@ fn materialize_reading( transaction.execute_batch( "CREATE TABLE IF NOT EXISTS view_bindings ( file_path TEXT PRIMARY KEY, - payload TEXT NOT NULL + payload BLOB NOT NULL ); CREATE TABLE IF NOT EXISTS view_file_surfaces ( file_path TEXT PRIMARY KEY, - payload TEXT NOT NULL + payload BLOB NOT NULL )", )?; transaction.execute_batch(module_digest::CREATE_TABLES)?; @@ -947,7 +948,11 @@ fn materialize_reading( .surface_json() .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))? { - surface_rows.push((path.clone(), payload, existing_surface_paths.contains(path))); + surface_rows.push(( + path.clone(), + payload::pack(&payload), + existing_surface_paths.contains(path), + )); } } let old = cached.get(path).filter(|_| { @@ -985,7 +990,7 @@ fn materialize_reading( let replaced = old.is_some() || existing_binding_paths.contains(path); let payload = serde_json::to_string(binding) .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; - binding_inserts.push((path.clone(), payload, replaced)); + binding_inserts.push((path.clone(), payload::pack(&payload), replaced)); } let surface_updates = surface_rows .iter() @@ -1755,14 +1760,15 @@ fn load_bindings( let mut statement = connection.prepare("SELECT file_path, payload FROM view_bindings")?; let rows = statement .query_map([], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + Ok((row.get::<_, String>(0)?, row.get::<_, Vec>(1)?)) })? .collect::, _>>()?; let mut bindings = rows .into_par_iter() .map(|(path, payload)| { - let binding: join::ViewBindingDependencies = serde_json::from_str(&payload) - .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; + let binding: join::ViewBindingDependencies = + serde_json::from_str(&payload::unpack(&payload)?) + .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; Ok((path, binding)) }) .collect::>>()?; @@ -1780,13 +1786,14 @@ fn load_surfaces( let mut statement = connection.prepare("SELECT file_path, payload FROM view_file_surfaces")?; let rows = statement .query_map([], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + Ok((row.get::<_, String>(0)?, row.get::<_, Vec>(1)?)) })? .collect::, _>>()?; rows.into_par_iter() .map(|(path, surface)| { - let binding = join::ViewBindingDependencies::from_surface_json(&surface) - .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; + let binding = + join::ViewBindingDependencies::from_surface_json(&payload::unpack(&surface)?) + .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; Ok((path, binding)) }) .collect() @@ -1826,14 +1833,15 @@ fn load_bindings_for_selection( )?; let rows = statement .query_map([], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + Ok((row.get::<_, String>(0)?, row.get::<_, Vec>(1)?)) })? .collect::, _>>()?; let selected = rows .into_par_iter() .map(|(path, payload)| { - let binding: join::ViewBindingDependencies = serde_json::from_str(&payload) - .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; + let binding: join::ViewBindingDependencies = + serde_json::from_str(&payload::unpack(&payload)?) + .map_err(|error| CallGraphStoreError::Unavailable(error.to_string()))?; Ok((path, binding)) }) .collect::>>()?; diff --git a/crates/aft/src/views/materialization/dispatch.rs b/crates/aft/src/views/materialization/dispatch.rs index 16f9971e6..55d9e4082 100644 --- a/crates/aft/src/views/materialization/dispatch.rs +++ b/crates/aft/src/views/materialization/dispatch.rs @@ -22,6 +22,8 @@ use crate::callgraph_store::join::{ BlobImport, ManifestBlobReader, ParseBlob, }; use crate::callgraph_store::{CallGraphStoreError, Result}; + +use super::payload; use crate::views::{Manifest, ManifestEntry}; /// What the method-call pass may assume about this transaction. @@ -221,16 +223,24 @@ fn type_target_key(dependency: &str) -> String { format!("t:{dependency}") } +// The two site tables are read only by their keys or in full into sets, so +// they keep no separate rowid copy of their primary key (`WITHOUT ROWID`). +// `view_unknown_live` holds, per caller file, the targets its unknown-receiver +// sites keep alive. Its only reader asks which targets are alive at all, and +// the writers replace a file's rows as a whole, so it keeps no site ordinal: a +// common call such as `.clone()` would otherwise store every `clone` target +// once per call site. The JSON columns of `view_dispatch_files` are stored +// compressed (see `payload`); the digests stay those of the JSON text. const CREATE_TABLES: &str = "CREATE TABLE IF NOT EXISTS view_dispatch_sites ( caller_file TEXT NOT NULL, ordinal INTEGER NOT NULL, language TEXT NOT NULL, unresolved_receiver_sites INTEGER NOT NULL, dynamic_member_sites INTEGER NOT NULL, - external INTEGER NOT NULL, PRIMARY KEY(caller_file, ordinal)); + external INTEGER NOT NULL, PRIMARY KEY(caller_file, ordinal)) WITHOUT ROWID; CREATE TABLE IF NOT EXISTS view_unknown_live ( - caller_file TEXT NOT NULL, ordinal INTEGER NOT NULL, target_file TEXT NOT NULL, - target_symbol TEXT NOT NULL, PRIMARY KEY(caller_file, ordinal, target_file, target_symbol)); + caller_file TEXT NOT NULL, target_file TEXT NOT NULL, target_symbol TEXT NOT NULL, + PRIMARY KEY(caller_file, target_file, target_symbol)) WITHOUT ROWID; CREATE TABLE IF NOT EXISTS view_dispatch_files ( file_path TEXT PRIMARY KEY, facts_digest TEXT NOT NULL, sites_digest TEXT NOT NULL, - facts TEXT NOT NULL, type_target_trace TEXT, sites TEXT NOT NULL); + facts BLOB NOT NULL, type_target_trace BLOB, sites BLOB NOT NULL); CREATE TABLE IF NOT EXISTS view_dispatch_dependencies ( file_path TEXT NOT NULL, dependency TEXT NOT NULL, PRIMARY KEY(file_path, dependency)) WITHOUT ROWID; @@ -271,9 +281,10 @@ pub(super) fn type_target_traces( .prepare_cached("SELECT type_target_trace FROM view_dispatch_files WHERE file_path=?1")?; let mut traces = BTreeMap::new(); for file in files { - let trace: Option> = read.query_row([&file], |row| row.get(0)).optional()?; + let trace: Option>> = + read.query_row([&file], |row| row.get(0)).optional()?; if let Some(Some(trace)) = trace { - traces.insert(file, from_json(&trace)?); + traces.insert(file, from_json(&payload::unpack(&trace)?)?); } } Ok(traces) @@ -412,8 +423,8 @@ fn load_sites<'p>( let Some(row) = rows.get_mut(file).filter(|row| row.sites.is_none()) else { continue; }; - let sites: String = query.query_row([file], |row| row.get(0))?; - row.sites = Some(from_json(&sites)?); + let sites: Vec = query.query_row([file], |row| row.get(0))?; + row.sites = Some(from_json(&payload::unpack(&sites)?)?); } Ok(()) } @@ -496,11 +507,12 @@ pub(super) fn emit_from_decoded( row.get::<_, String>(0)?, row.get::<_, String>(1)?, row.get::<_, String>(2)?, - row.get::<_, String>(3)?, + row.get::<_, Vec>(3)?, )) })? .collect::, _>>()?; - for (path, facts_digest, sites_digest, facts_json) in stored { + for (path, facts_digest, sites_digest, facts) in stored { + let facts_json = payload::unpack(&facts)?; if changed.contains(&path) { touched.insert(path.clone()); } @@ -859,19 +871,27 @@ fn persist( let mut delete_dependency = transaction .prepare("DELETE FROM view_dispatch_dependencies WHERE file_path=?1 AND dependency=?2")?; for (path, row) in rows { - let trace = row.trace.as_ref().map(json).transpose()?; + let trace = row + .trace + .as_ref() + .map(|trace| json(trace).map(|trace| payload::pack(&trace))) + .transpose()?; if let Some(sites) = &row.sites_json { replace.execute(params![ path, row.facts_digest, row.sites_digest, - row.facts_json, + payload::pack(&row.facts_json), trace, - sites + payload::pack(sites) ])?; } else { if row.dirty { - update_facts.execute(params![path, row.facts_digest, row.facts_json])?; + update_facts.execute(params![ + path, + row.facts_digest, + payload::pack(&row.facts_json) + ])?; } if trace.is_some() { update_trace.execute(params![path, trace])?; @@ -965,8 +985,8 @@ fn write_row_by_row( )?; for (target_file, target_symbol) in &resolution.protected { transaction.execute( - "INSERT INTO view_unknown_live VALUES (?1,?2,?3,?4)", - params![site.file, site.site.ordinal, target_file, target_symbol], + "INSERT OR IGNORE INTO view_unknown_live VALUES (?1,?2,?3)", + params![site.file, target_file, target_symbol], )?; } } @@ -1128,7 +1148,7 @@ fn write_differences( } } - // Unknown-receiver liveness: one row per site and protected target. + // Unknown-receiver liveness: one row per caller file and protected target. { let empty: bool = transaction.query_row( "SELECT NOT EXISTS(SELECT 1 FROM view_unknown_live)", @@ -1136,38 +1156,32 @@ fn write_differences( |row| row.get(0), )?; if empty { - // A fresh graph has no old protection rows to reconcile. Sites - // have unique ordinals here and each protected set is deduplicated, - // so stream the rows without cloning the full set of string keys. + // A fresh graph has no old protection rows to reconcile, so stream + // the rows without cloning the full set of string keys. Several + // sites of one file can protect the same target; the table keeps + // one row per file and target. let mut insert = - transaction.prepare("INSERT INTO view_unknown_live VALUES (?1,?2,?3,?4)")?; + transaction.prepare("INSERT OR IGNORE INTO view_unknown_live VALUES (?1,?2,?3)")?; for site in sites { for (file, symbol) in &site.resolution.protected { counts.unknown_live_inserted += - insert.execute(params![site.file, site.site.ordinal, file, symbol])?; + insert.execute(params![site.file, file, symbol])?; } } } else { - let mut wanted = BTreeSet::<(String, i64, String, String)>::new(); + let mut wanted = BTreeSet::<(String, String, String)>::new(); for site in sites { for (file, symbol) in &site.resolution.protected { - wanted.insert(( - site.file.to_string(), - i64::from(site.site.ordinal), - file.clone(), - symbol.clone(), - )); + wanted.insert((site.file.to_string(), file.clone(), symbol.clone())); } } let mut stale = Vec::new(); - let columns = - "SELECT caller_file, ordinal, target_file, target_symbol FROM view_unknown_live"; + let columns = "SELECT caller_file, target_file, target_symbol FROM view_unknown_live"; let read_row = |row: &rusqlite::Row<'_>| { Ok(( row.get::<_, String>(0)?, - row.get::<_, i64>(1)?, + row.get::<_, String>(1)?, row.get::<_, String>(2)?, - row.get::<_, String>(3)?, )) }; let mut read = transaction.prepare(&match files { @@ -1188,17 +1202,15 @@ fn write_differences( } let mut delete = transaction.prepare( "DELETE FROM view_unknown_live - WHERE caller_file=?1 AND ordinal=?2 AND target_file=?3 AND target_symbol=?4", + WHERE caller_file=?1 AND target_file=?2 AND target_symbol=?3", )?; for row in &stale { - counts.unknown_live_deleted += - delete.execute(params![row.0, row.1, row.2, row.3])?; + counts.unknown_live_deleted += delete.execute(params![row.0, row.1, row.2])?; } let mut insert = - transaction.prepare("INSERT INTO view_unknown_live VALUES (?1,?2,?3,?4)")?; + transaction.prepare("INSERT INTO view_unknown_live VALUES (?1,?2,?3)")?; for row in &wanted { - counts.unknown_live_inserted += - insert.execute(params![row.0, row.1, row.2, row.3])?; + counts.unknown_live_inserted += insert.execute(params![row.0, row.1, row.2])?; } } } diff --git a/crates/aft/src/views/materialization/dispatch_proofs.rs b/crates/aft/src/views/materialization/dispatch_proofs.rs index 7a58f1931..68f0f65bf 100644 --- a/crates/aft/src/views/materialization/dispatch_proofs.rs +++ b/crates/aft/src/views/materialization/dispatch_proofs.rs @@ -46,10 +46,14 @@ fn typescript_object_unknown_liveness_stream_matches_reference_and_refresh() { }) .unwrap() }; - assert_eq!(count(&streamed), 4); + // One row per caller file and protected target: both `x.run()` sites of + // caller.ts protect A.run and B.run, so two rows, not one per site. + assert_eq!(count(&streamed), 2); apply_manifest_diff(&streamed, &base, &next, &blob_path).unwrap(); let cold = dir.path().join("cold-next.sqlite"); materialize_manifest_view_database(&cold, &blob_path, &next).unwrap(); + // Removing one of the two sites keeps both rows: the remaining site still + // protects the same targets. assert_eq!(count(&streamed), 2); parity::assert_logical_parity(&streamed, &cold); } diff --git a/crates/aft/src/views/materialization/fact_tests.rs b/crates/aft/src/views/materialization/fact_tests.rs index c3779e6b0..0ee7d737a 100644 --- a/crates/aft/src/views/materialization/fact_tests.rs +++ b/crates/aft/src/views/materialization/fact_tests.rs @@ -307,7 +307,9 @@ fn unattributed_binding_forces_counted_full_fallback() { bindings.get_mut("a.ts").unwrap().unattributed = true; db.execute( "UPDATE view_bindings SET payload=?1 WHERE file_path='a.ts'", - [serde_json::to_string(&bindings["a.ts"]).unwrap()], + [super::payload::pack( + &serde_json::to_string(&bindings["a.ts"]).unwrap(), + )], ) .unwrap(); let started = std::time::Instant::now(); diff --git a/crates/aft/src/views/materialization/payload.rs b/crates/aft/src/views/materialization/payload.rs new file mode 100644 index 000000000..34d7d238f --- /dev/null +++ b/crates/aft/src/views/materialization/payload.rs @@ -0,0 +1,73 @@ +//! Stored form of the call-graph view's per-file JSON payloads. +//! +//! Resolver surfaces and bindings (`view_file_surfaces`, `view_bindings`) and +//! method-call facts, sites and type target traces (`view_dispatch_files`) +//! are repetitive JSON text: before compression they were about 300 MB of the +//! 1.9 GB call-graph base built for AFT's own repository. They are stored +//! zlib-compressed at the fastest level: a cold build packs every payload, and +//! for this text the fastest level costs a few percent more bytes than the +//! default for a fraction of the CPU. The zlib checksum makes a damaged row +//! fail to read rather than decode to different text. Everything in memory, including the digests +//! computed over the JSON text, stays the JSON text; only the stored bytes +//! change. + +use std::io::{Read, Write}; + +use flate2::read::ZlibDecoder; +use flate2::write::ZlibEncoder; +use flate2::Compression; + +use crate::callgraph_store::{CallGraphStoreError, Result}; + +/// The stored bytes of a JSON payload. +pub(crate) fn pack(json: &str) -> Vec { + let mut encoder = + ZlibEncoder::new(Vec::with_capacity(json.len() / 4 + 16), Compression::fast()); + encoder + .write_all(json.as_bytes()) + .expect("writing into a Vec cannot fail"); + encoder.finish().expect("writing into a Vec cannot fail") +} + +/// The JSON text of stored payload bytes. +pub(crate) fn unpack(bytes: &[u8]) -> Result { + let mut json = String::new(); + ZlibDecoder::new(bytes) + .read_to_string(&mut json) + .map_err(|error| { + CallGraphStoreError::Unavailable(format!("stored view payload is unreadable: {error}")) + })?; + Ok(json) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn payloads_round_trip_and_shrink() { + let json = serde_json::to_string( + &(0..200) + .map(|n| serde_json::json!({"path": "src/views/materialization.rs", "line": n})) + .collect::>(), + ) + .unwrap(); + let packed = pack(&json); + assert!( + packed.len() * 4 < json.len(), + "{} -> {}", + json.len(), + packed.len() + ); + assert_eq!(unpack(&packed).unwrap(), json); + assert_eq!(unpack(&pack("")).unwrap(), ""); + } + + #[test] + fn unreadable_payloads_are_refused() { + assert!(matches!( + unpack(b"{\"not\":\"zlib\"}"), + Err(CallGraphStoreError::Unavailable(_)) + )); + } +} diff --git a/crates/aft/src/views/materialization/tests.rs b/crates/aft/src/views/materialization/tests.rs index 220cfe365..d61554fad 100644 --- a/crates/aft/src/views/materialization/tests.rs +++ b/crates/aft/src/views/materialization/tests.rs @@ -451,7 +451,8 @@ fn selected_binding_load_skips_changed_payloads_and_matches_cold() { Connection::open(&incremental) .unwrap() .execute( - "UPDATE view_bindings SET payload = '{' WHERE file_path = 'target.ts'", + // An unreadable stored payload: the selected load must never read it. + "UPDATE view_bindings SET payload = X'7b' WHERE file_path = 'target.ts'", [], ) .unwrap(); diff --git a/crates/aft/src/views/worker_seed_tests.rs b/crates/aft/src/views/worker_seed_tests.rs index f0d3b6f88..f44380ebf 100644 --- a/crates/aft/src/views/worker_seed_tests.rs +++ b/crates/aft/src/views/worker_seed_tests.rs @@ -735,8 +735,11 @@ fn bench_repository_cold_materialization() { use std::process::{Command, Stdio}; let project = tempfile::tempdir().unwrap(); let storage = tempfile::tempdir().unwrap(); + // The whole repository, not only this crate. + let repository = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../.."); let archive = Command::new("git") .args(["archive", "HEAD"]) + .current_dir(&repository) .output() .unwrap(); assert!(archive.status.success()); @@ -781,30 +784,110 @@ fn bench_repository_cold_materialization() { callgraph: true, }; crate::blob_store::PAYLOAD_READ_COUNTS.with(|counts| counts.set(Default::default())); - let prepared = prepare_checkout(&request, &mut |_| Ok(())).unwrap(); + let mut prepared = prepare_checkout(&request, &mut |_| Ok(())).unwrap(); let reads = crate::blob_store::PAYLOAD_READ_COUNTS.with(|counts| counts.get()); - let (path, connection) = prepared.derived_checkpoint.as_ref().unwrap(); + let files = prepared.report.manifest.as_ref().unwrap().entries().count(); + let materialize_ms = prepared.profile.materialization_call_ms; + prepared.commit().unwrap(); + drop(prepared); + // The published database's checkpoint runs shortly after publication; + // measure only once nothing else holds the file. + std::thread::sleep(std::time::Duration::from_secs(3)); + fn derived_databases(directory: &Path, found: &mut Vec) { + for entry in fs::read_dir(directory).unwrap().flatten() { + let path = entry.path(); + if path.is_dir() { + derived_databases(&path, found); + } else if path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with("derived-") && name.ends_with(".sqlite")) + { + found.push(path); + } + } + } + let mut found = Vec::new(); + derived_databases(storage.path(), &mut found); + let path = found + .into_iter() + .max_by_key(|path| fs::metadata(path).map(|meta| meta.len()).unwrap_or(0)) + .expect("a published derived database"); + let connection = rusqlite::Connection::open(&path).unwrap(); + connection + .query_row("PRAGMA wal_checkpoint(TRUNCATE)", [], |_| Ok(())) + .unwrap(); + let bytes: i64 = connection + .query_row( + "SELECT page_count * page_size FROM pragma_page_count(), pragma_page_size()", + [], + |row| row.get(0), + ) + .unwrap(); let unknown: usize = connection .query_row("SELECT COUNT(*) FROM view_unknown_live", [], |row| { row.get(0) }) .unwrap(); - println!("repository files={} materialize_ms={} derived_bytes={} unknown_live_rows={} payload_reads={} payload_bytes={}", prepared.report.manifest.as_ref().unwrap().entries().count(), prepared.profile.materialization_call_ms, fs::metadata(path).unwrap().len(), unknown, reads.blake3_calls, reads.bytes); - for name in [ + println!("repository files={files} materialize_ms={materialize_ms} derived_bytes={bytes} unknown_live_rows={unknown} payload_reads={} payload_bytes={}", reads.blake3_calls, reads.bytes); + let mut tables = connection + .prepare("SELECT name, SUM(pgsize) FROM dbstat GROUP BY name ORDER BY 2 DESC") + .unwrap(); + for row in tables + .query_map([], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) + }) + .unwrap() + { + let (name, bytes) = row.unwrap(); + println!("repository table={name} allocated_bytes={bytes}"); + } + // Stored bytes per column of the largest tables, to choose what to + // encode next from measurement rather than estimates. + for table in [ "refs", "edges", - "view_bindings", - "view_unknown_live", - "sqlite_autoindex_view_unknown_live_1", + "nodes", + "file_dependencies", + "view_dispatch_dependencies", + "view_dispatch_sites", ] { - let bytes: i64 = connection - .query_row( - "SELECT COALESCE(SUM(pgsize),0) FROM dbstat WHERE name=?1", - [name], - |row| row.get(0), - ) + let columns: Vec = connection + .prepare(&format!("SELECT name FROM pragma_table_info('{table}')")) + .unwrap() + .query_map([], |row| row.get(0)) + .unwrap() + .collect::>() .unwrap(); - println!("repository table={name} allocated_bytes={bytes}"); + for column in columns { + let (rows, stored, distinct): (i64, i64, i64) = connection + .query_row( + &format!( + "SELECT COUNT(*), COALESCE(SUM(LENGTH(CAST(\"{column}\" AS BLOB))), 0), COUNT(DISTINCT \"{column}\") FROM \"{table}\"" + ), + [], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), + ) + .unwrap(); + println!( + "repository column={table}.{column} rows={rows} stored_bytes={stored} distinct={distinct}" + ); + } + } + let indexes: Vec<(String, String)> = connection + .prepare( + "SELECT name, COALESCE(sql, '') FROM sqlite_master WHERE type = 'index' ORDER BY name", + ) + .unwrap() + .query_map([], |row| Ok((row.get(0)?, row.get(1)?))) + .unwrap() + .collect::>() + .unwrap(); + for (name, sql) in indexes { + println!( + "repository index={name} sql={}", + sql.split_whitespace().collect::>().join(" ") + ); } } diff --git a/crates/aft/tests/store_diet/baseline-plans.json.gz b/crates/aft/tests/store_diet/baseline-plans.json.gz index 264b8c19d..ae384666e 100644 Binary files a/crates/aft/tests/store_diet/baseline-plans.json.gz and b/crates/aft/tests/store_diet/baseline-plans.json.gz differ diff --git a/crates/aft/tests/store_diet/corpus-views.json.gz b/crates/aft/tests/store_diet/corpus-views.json.gz index 318311855..a8b417f2e 100644 Binary files a/crates/aft/tests/store_diet/corpus-views.json.gz and b/crates/aft/tests/store_diet/corpus-views.json.gz differ diff --git a/crates/aft/tests/store_diet/harness.rs b/crates/aft/tests/store_diet/harness.rs index 80210236f..7fc2e6f92 100644 --- a/crates/aft/tests/store_diet/harness.rs +++ b/crates/aft/tests/store_diet/harness.rs @@ -97,7 +97,17 @@ fn golden(name: &str, bytes: &[u8]) { }); write_capture(&path, bytes); } else { - assert_eq!(read_capture(&path), bytes, "{} differs", path.display()); + let recorded = read_capture(&path); + // A candidate's differing capture can be kept for review (never as a + // golden): AFT_STORE_DIET_CANDIDATE_DIR names a folder to write it to. + if recorded != bytes { + if let Some(directory) = std::env::var_os("AFT_STORE_DIET_CANDIDATE_DIR") { + let directory = PathBuf::from(directory); + fs::create_dir_all(&directory).unwrap(); + fs::write(directory.join(name.trim_end_matches(".gz")), bytes).unwrap(); + } + } + assert_eq!(recorded, bytes, "{} differs", path.display()); } } @@ -1263,9 +1273,17 @@ fn stored_json_facts(store: &CallGraphStore) -> Value { "SELECT {key}, {column} FROM {name} WHERE {column} IS NOT NULL" )) .unwrap(); + // Payload columns hold JSON text, or the views store's compressed + // form of it; either way the facts compared are the decoded JSON. let rows = statement .query_map([], |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + let payload = match row.get_ref(1)? { + rusqlite::types::ValueRef::Blob(bytes) => { + crate::views::materialization::payload::unpack(bytes).unwrap() + } + other => other.as_str()?.to_string(), + }; + Ok((row.get::<_, String>(0)?, payload)) }) .unwrap(); for row in rows { diff --git a/crates/aft/tests/store_diet/order_sql.json.gz b/crates/aft/tests/store_diet/order_sql.json.gz index eb38468bf..7e72aa02f 100644 Binary files a/crates/aft/tests/store_diet/order_sql.json.gz and b/crates/aft/tests/store_diet/order_sql.json.gz differ diff --git a/crates/aft/tests/store_diet/source_inventory.json.gz b/crates/aft/tests/store_diet/source_inventory.json.gz index 05aa0d762..2e587c50f 100644 Binary files a/crates/aft/tests/store_diet/source_inventory.json.gz and b/crates/aft/tests/store_diet/source_inventory.json.gz differ diff --git a/crates/aft/tests/store_diet/sqlite-order.json b/crates/aft/tests/store_diet/sqlite-order.json index eb1271328..5654e28fc 100644 --- a/crates/aft/tests/store_diet/sqlite-order.json +++ b/crates/aft/tests/store_diet/sqlite-order.json @@ -35,13 +35,13 @@ "dispatch_node_kind": "class", "ambiguity_candidates": [ { - "id": "pos:symbol:227ce43cf7199dbd8b5218c388da7fb30b3396fd6482be54dd117e749e9931d2", + "id": "pos:symbol:227ce43cf7199dbd", "kind": "class", "name": "f", "line": 1 }, { - "id": "pos:symbol:73c5f1d7de9bbb01006c2adf5c30794d989e5b8a632ca0615b573e7c0338be09", + "id": "pos:symbol:73c5f1d7de9bbb01", "kind": "function", "name": "f", "line": 1 @@ -53,13 +53,13 @@ "dispatch_node_kind": "function", "ambiguity_candidates": [ { - "id": "pos:symbol:73c5f1d7de9bbb01006c2adf5c30794d989e5b8a632ca0615b573e7c0338be09", + "id": "pos:symbol:73c5f1d7de9bbb01", "kind": "function", "name": "f", "line": 1 }, { - "id": "pos:symbol:227ce43cf7199dbd8b5218c388da7fb30b3396fd6482be54dd117e749e9931d2", + "id": "pos:symbol:227ce43cf7199dbd", "kind": "class", "name": "f", "line": 1 diff --git a/crates/aft/tests/store_diet/statements.json.gz b/crates/aft/tests/store_diet/statements.json.gz index a968563ae..59151bae9 100644 Binary files a/crates/aft/tests/store_diet/statements.json.gz and b/crates/aft/tests/store_diet/statements.json.gz differ diff --git a/crates/aft/tests/store_diet/stored-shape-values.json b/crates/aft/tests/store_diet/stored-shape-values.json index 712b7ff49..bee70f2ae 100644 --- a/crates/aft/tests/store_diet/stored-shape-values.json +++ b/crates/aft/tests/store_diet/stored-shape-values.json @@ -82,7 +82,7 @@ "typescript": 5 }, "nodes.id_form": { - "view:symbol:pos:symbol:": 3 + "digest": 3 }, "refs.id_form": { "view:": 6 diff --git a/crates/aft/tests/store_diet/sweep_references.json b/crates/aft/tests/store_diet/sweep_references.json index c3141afe0..31027c258 100644 --- a/crates/aft/tests/store_diet/sweep_references.json +++ b/crates/aft/tests/store_diet/sweep_references.json @@ -16,8 +16,8 @@ { "table": "dispatch_hints", "column": "method_name", - "index": "idx_dispatch_hints_method", - "new_index": false, + "index": "diet_sweep_dispatch_hints_method_name", + "new_index": true, "probe": "SELECT 1 FROM dispatch_hints WHERE method_name = ?1 LIMIT 1" }, { diff --git a/crates/aft/tests/store_diet/table_encoding.json.gz b/crates/aft/tests/store_diet/table_encoding.json.gz index 18c427c44..bab9af59e 100644 Binary files a/crates/aft/tests/store_diet/table_encoding.json.gz and b/crates/aft/tests/store_diet/table_encoding.json.gz differ