Compute the crate hash from encoded metadata in non-incremental builds [-1.00% non-incr full] - #61
Draft
xmakro wants to merge 3 commits into
Draft
Compute the crate hash from encoded metadata in non-incremental builds [-1.00% non-incr full]#61xmakro wants to merge 3 commits into
xmakro wants to merge 3 commits into
Conversation
xmakro
changed the title
The crate hash was computed by flushing the encoder, seeking back to the start of the file and reading every byte of it again through a scratch buffer. Feed the bytes to the hasher as they leave the write buffer instead, using the flush strategy `FileEncoder` already supports. That drops the read-back and hashes the bytes while they are still in cache. The hash is only known once the file is complete, so `encode_crate_root` encodes a zeroed placeholder and `with_encode_metadata_header` patches the real hash over it afterwards, the same way the root position is already patched. To give the placeholder a fixed offset, `hash` becomes the first field of `CrateHeader`, which is itself the first field of `CrateRoot`, so it lands exactly at the root position. The patch reads the placeholder back and asserts it is still zeroed, so a later field reorder fails loudly rather than corrupting the metadata. The hashed range stops before the root, which is where it stopped before. The root holds `extra_filename`, which must stay out of the crate hash: `-Cextra-filename` is untracked, and two rlibs built from the same source that differ only by it have to be recognized as the same crate.
The hash covered the encoded contents but not the `CrateRoot`, so a few things it holds went unhashed. Cover them in the two ways that keep the crate hash meaning what it meant before. `hash_crate_root` hashes the root's own fields. It destructures the root exhaustively, so a new field has to be classified before it compiles. Most fields need nothing: the `Lazy*` ones are positions into contents that are already hashed, and the rest are derived from tracked command line options. What is left, and what this fixes, are the crate level attribute flags such as `compiler_builtins`, `needs_allocator` and `has_global_allocator`. Those attributes are `EncodeCrossCrate::No`, so they appear nowhere else in the metadata. `extra_filename` is skipped; it must not affect crate identity. The non-HIR components of `tcx.crate_hash` are folded in as well. The encoded bytes do not carry the tracked command line options, so without this the hash no longer changed when, for instance, an output type was added, which tests/run-make/emit-path-unhashed requires. It also means only the `hir_body_hash` component is replaced by the metadata bytes; everything else the crate hash covered, it still covers. tests/ui 21309 passed, tests/run-make 374 passed, tests/incremental 178 passed.
xmakro
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The crate hash is computed by stable-hashing the HIR. When nothing but the metadata needs it, hash the encoded metadata bytes instead: they are what downstream crates actually read, and they are already being written out.
-1.00% on non-incr full measurements (syn check -2.43%, serde check -2.29%, hyper check -1.98%, cargo check -1.87%), 7 cells >=1%, 0 regressions, max-rss -0.24%. tests/ui 21309 passed, tests/run-make 374 passed, tests/incremental 178 passed, 0 failed.
The metadata bytes replace only the
hir_body_hashcomponent, so the hash is assembled from three parts:FileEncoderalready supports a flush strategy, so this costs no extra pass over the file.hash_crate_root, for theCrateRootfields that appear nowhere else. Most of the root needs nothing: theLazy*fields are positions into contents that are already hashed, and the rest is derived from tracked command line options. What is left are the crate level attribute flags such ascompiler_builtins,needs_allocatorandhas_global_allocator, whose attributes areEncodeCrossCrate::No. The root is destructured exhaustively, so a new field has to be classified before it compiles.tcx.crate_hash: tracked command line options, upstream crate hashes, remapped source file names and the crate's own stable id. Without these the hash stopped changing when an output type was added, which tests/run-make/emit-path-unhashed requires.extra_filenameis deliberately left out of all three.-Cextra-filenameis untracked, and two rlibs built from the same source that differ only by it have to be recognized as the same crate (tests/run-make/extern-flag-fun).The hash is only known once the file is complete, so the root is encoded with a zeroed placeholder that is patched afterwards, the same way the root position already is.
METADATA_VERSIONgoes 10 -> 11 becausehashbecomes the first field ofCrateHeaderto give that placeholder a fixed offset; the patch reads it back and asserts it is still zeroed, so a later field reorder fails loudly rather than corrupting the metadata.Proc-macro crates keep the HIR hash, since their metadata is a stub.
Worth knowing for review:
needs_hir_hash()includescfg!(debug_assertions), so none of this path runs on a compiler built with assertions.