Represent spans as (file, offset, length) in stable hashing and make line-number observation a tracked dependency - #91
Represent spans as (file, offset, length) in stable hashing and make line-number observation a tracked dependency#91xmakro wants to merge 1 commit into
Conversation
969e10c to
6ee0c86
Compare
6ee0c86 to
b378bf7
Compare
e3ebdc2 to
46b20ce
Compare
d00cac2 to
d9c211d
Compare
d9c211d to
deca580
Compare
|
Closing in favor of #99. The def-anchored channel measures parity or better on the full suite (the last same-day A/B put it at -0.04% net against this PR's head with no cell differing by 0.25% either way) with strictly less machinery: no SelfHash key style, no truncated file-id index and collision abort, no import-all-crates fallback, no bucket arithmetic (the class that shipped an off-by-one once). The deciding structural point: #97's def_position query keys by definition, and on a per-definition line channel the two queries fuse into the single def_anchor node, one eval_always node per rendered definition instead of two. The bucketed key space cannot express that fusion. #97 has been rebased onto #99 accordingly and its def_position query is deleted; same-day numbers for the fused stack are in the two PR bodies. Reworking #98's expansion call-site anchors onto the new base is the remaining follow-up. |
Span hashing and the
TAG_FULL_SPANcache encoding both represented a span as (file, line, column), paying line-table lookups for every hashed or cached span. Both now use (file, offset in file, length):stable_hash_spanneeds no line-table work at all,TAG_FULL_SPANdecoding reconstructs the exact span (so it re-hashes to the fingerprint its containing value was stored under), andCachingSourceMapViewis deleted.The reason the old scheme hashed line and column was not the span itself: a span's position is fully captured by (file, offset, length). It was protecting data derived from spans and baked into cached artifacts, namely the line/column numbers in
#[track_caller]locations, debuginfo line tables, coverage mappings, and pretty-printed type paths embedded in cached diagnostics. Those reads go through the untrackedSourceMap, so the dep graph never sees them, and upstream compensates by smearing line/column sensitivity into every span fingerprint, taxing the vast majority of consumers that never look at a line number. (The protection was also incomplete: an operator-precedence bug in the truncated packing,(col_hi.0 as u64) & 0xFF << 32, meant the end column was never actually hashed. The new scheme covers end positions exactly, since (offset, length) determines them.)This PR replaces the smearing with an explicit dependency. A new
eval_alwaysquery,file_lines_prefix_hash(LineTablePrefixKey), hashes a prefix of a file's line-start table (through the end of a 64-line bucket, plus the multibyte-character and position-normalization tables). Line lookup for a position on line L is a partition point over the table: it reads line starts up to L and also entry L + 1, which bounds the line from above, so the recorded key covers the prefix through the bucket of entry L + 1 and stays green when line breaks only move after that prefix. (An earlier revision keyed the bucket of L itself; when L was the last line of a bucket, an offset-preserving edit inserting a line start exactly at the boundary changed the line mapping while the covered prefix stayed identical, replaying a stale caller line.tests/run-make/incr-line-table-bucket-boundarypins this, and unit tests inrustc_spanpin the prefix/clamping properties.) All bucket hashes for a file are snapshots of one running hash, computed in a single O(n) pass and cached on theSourceFile, so the query is O(1) per bucket. The query is not read directly:TyCtxt::lookup_line_tracked(pos)returns the file and line while recording the matching dependency, so the data and the dep edge cannot be separated, and the untrackedSourceMaplookups document when they must not be used. Every site that renders line/column data into a cached artifact derives it from a tracked lookup:TyCtxt::span_as_caller_location, the funnel for#[track_caller]in both const eval and codegen, andlocation_triple_for_spanin the const-eval machine, which renders positions into const-panic messages (today those only reach hard errors, which are never replayed from cache; it uses the tracked lookup anyway so it cannot silently go stale if that changes),lookup_debug_locin cg_llvm and cg_gcc, andget_span_locin cg_clif, the funnels for debuginfo line tables (AOT codegen in all three backends runs insidedep_graph.with_task, so the read records a real edge; cg_clif's JIT mode runs outside any task and drops the read, which is fine because JIT caches no artifacts across sessions),mapgen, which stores line/column coordinates for positions throughout the file and therefore uses the whole-table variantTyCtxt::source_file_tracked(make_coordsnow also rejects spans that leave the mapped file, instead of rendering them against the wrong table),{closure@file:line:col},{coroutine@...}and<impl at file:line:col>forms, viaTyCtxt::span_to_diagnostic_string_tracked. These strings become diagnostic arguments, and warnings emitted inside queries are cached as side effects and replayed verbatim when the query is green, so they are cached artifacts like any other. (Hard errors never replay: error sessions skip incremental cache finalization.)Other line observations were audited and need no edge: the remaining parser and
rustc_passesuses feed hard errors, andline!()/column!()/file!(), the proc-macro server, and test-harness location literals run at expansion time, which re-executes every session and feeds content-hashed tokens downstream. Those sites now carry comments stating that invariant, because caching expansion output across sessions would void it.Invalidation precision improves on both schemes considered before:
file_lines_prefix_hashchanges and exactly the line-observing nodes (codegen of that file's items, caller-location consts, line-rendering diagnostics) re-run.tests/run-make/incr-track-caller-line-shiftbuilds a byte-offset-identical edit pair and checks the runtime#[track_caller]line,DW_AT_decl_linein the DWARF, and, viarustc_partition_reused/rustc_partition_codegened, that the invalidation is targeted: the byte-identical line-shifted module is re-codegened while a module in an earlier line-table bucket is reused.constthat capturesLocation::caller()genuinely depends on its line number and must re-evaluate); everything else in a check build is insensitive to the query.Scope: the dependency covers line indices and character columns (the line-start, multibyte-character and position-normalization tables;
tests/run-make/incr-multibyte-caller-columnpins the table coverage). It does not cover display columns, which additionally read the rendered line's text for character widths; a byte-identical edit such as tab-to-space can leave a stale display column in a caller location. The old line/column hashing had exactly the same limitation (it hashed byte columns), so this is inherited, not introduced; the doc comments scope the guarantee explicitly. Under-Zincremental-ignore-spansthe line-table deps are still recorded, which is the sound direction (extra invalidation), while that flag's usual staleness caveats continue to apply. The new dep node cannot be named in#[rustc_clean]/#[rustc_dirty]attributes (its key is not def-path-based); the run-make tests userustc_partition_*assertions instead.Compared to the previous line/column scheme the remaining granularity difference is that a line-structure change invalidates the line-observing nodes for the whole 64-line bucket prefix rather than only those after the edit point, and offset shifts invalidate absolute spans that line/column happened to preserve; in exchange, span hashing does no line-table work anywhere.
Edge behavior: empty files contain no positions (their spans hash invalid and encode partial), an empty source map is handled, and spans crossing a file boundary hash invalid and also encode partial, so the cache never stores an exact span whose position no fingerprint covers. Full-span decoding asserts (unconditionally, not just in debug builds) that the decoded range, start plus length, is in bounds for the file: an out-of-range range means stale data referencing a shrunk file, and a silent misdecode would fabricate a position inside the next file. The 64-bit truncated file id used to key dep nodes is resolved through an index that is only maintained when incremental compilation is enabled; a truncation collision (which would silently alias two files' line tables) aborts at registration, and only incremental sessions are exposed to it. A key whose file no longer exists resolves to
None, which cannot match any previous hash, so dependents of deleted files re-execute.tests/run-make/incr-foreign-line-table-deppins the remaining lifecycle case: dep nodes recorded for upstream files (debuginfo of locally instantiated#[inline]/generic upstream code) are forced during try-mark-green before any span pointing into the upstream file has been decoded, so the provider imports upstream file tables on a miss; without that, such nodes count as changed on every rebuild and all reuse is lost. All test fixtures are generated by theirrmake.rs(nothing byte-sensitive is checked in, so rustfmt needs no exceptions), and each test asserts its offset-parity invariant before building.Performance
Local A/B at this head against the merge-base (3659db0), measured the same day with the same config on both sides (instructions:u, jemalloc, stage2 builds without PGO/LTO/BOLT) over regex-automata/syn/serde/cargo/ripgrep/hyper, Check/Debug/Opt, all scenarios; 81 cells total (18 crate-profile cells per scenario plus the extra incr-patched variants). Geomean of per-cell ratios:
45 of 81 cells improved by at least 0.25%; none regressed by 0.25% or more. Check incr-unchanged improves 0.6% to 0.9% on every crate. These numbers are for this PR alone at its current revision, so all of the correctness hardening above is included; the one regression an earlier revision had (cargo debug incr-full +0.32%) is no longer present.
Two earlier revisions document why the design looks the way it does. Hashing the whole file's line table per dependency (no buckets) put ripgrep opt incr-patched at +39%: the println patch lands mid-file in
crates/core/search.rs, whose generic functions are inlined across many CGUs, and every CGU baking a#[track_caller]location from that file was re-codegenned; the bucketed prefix restores after-the-edit granularity and that cell is now under 0.25%. Before that, a revision without on-demand import of upstream file tables hit +82%/+465% on cargo/ripgrep debug and opt because dep nodes recorded for not-yet-imported foreign files could not be re-executed during marking; the provider now imports upstream file tables on a miss.The query is skipped entirely when the dep graph is disabled, so non-incremental builds only benefit from the removed line-table lookups in span hashing.