Investigate computing crate_hash by hashing metadata

[cjgillot]

The crate_hash query is used to detect is metadata has changed before attempting to decode more complex information from it.
Until now, this crate hash has been computed manually using HIR.
Computing it while encoding metadata itself may be more robust and exhaustive.

General instructions:

• add a StableHasher and a StableHashingContext in rustc_metadata::rmeta::encoder::EncodeContext;
• double all accesses to the embedded opaque encoder by a call to hash_stable with the same data;
• manually reimplement encoding and hashing for special datatypes (CrateNum, DefId, LocalDefId...);
• remove the implementation of the crate_hash query in rustc_middle::hir::map, and reimplement it from the just-encoded metadata;
• evaluate effect on performance.

Please reach out on zulip if more details are required.
cc @michaelwoerister

[michaelwoerister]

I think this is very promising. I would suggest to entirely remove the local version of the crate_hash query. That should make things easier to implement.

[lqd]

@rustbot claim

[bjorn3]

The crate metadata is missing the session configuration with which the crate has been compiled. This means that eg changing optimization level probably won't change crate metadata. It probably needs to change the crate hash though. Including tcx.sess.opts.dep_tracking_hash(true) should be enough:

rust/compiler/rustc_middle/src/hir/map/mod.rs

Line 1135 in 4e82f35

tcx.sess.opts.dep_tracking_hash(true).hash_stable(&mut hcx, &mut stable_hasher);

[michaelwoerister]

Ideally it wouldn't be necessary because the hash includes everything that is accessible to downstream crates, right?

[bjorn3]

The object files are accessible to downstream crates (they are linked into the final artifact after all), but they depend on both the crate metadata and the compilation session options.

[michaelwoerister]

Yes, but linking is done unconditionally, independent of the crate hash.

[bjorn3]

I think it isn't too far fetched that cargo or other build systems will use the crate hash in the far future instead of mtime or hashing the entire file.

[michaelwoerister]

I don't know. In my opinion the crate hash is an internal implementation detail of the incremental compilation system that's only there to optimize a common code path there. I wouldn't recommend any outside tooling to rely on it.

[bjorn3]

The crate hash existed long before incremental compilation was introduced afaik.

[bjorn3]

The SVH has introduced feb 2014 in ec57db0

The goal of this hash is to identify when upstream crates have changed but
downstream crates have not been recompiled.

[lqd]

(I've begun looking at this already, but I've unfortunately realized that I won't have time to work on this soon enough.

Since it's both short-to-try and has mentorship available from michael and camille, it should be a good opportunity for a new contributor, rather than waiting on me and taking this opportunity away from someone who's interested.

@rustbot release-assignment
)

[PrestonFrom]

I'm still pretty new, but I'd like to take a stab at this. If it would be more appropriate for something with more experience, though, I'm happy to be un-assigned.

@rustbot claim

[PrestonFrom]

I've gotten very busy all the sudden -- I'm going to unassign myself in case someone has bandwidth to grab this now.

[kartva]

@rustbot claim

[Enselic]

Triage: Are you still working on this @DesmondWillowbrook ?

[pierwill]

I might be interested in taking this on :)

…

On Sat, Mar 16, 2024 at 12:07 PM Martin Nordholts ***@***.***> wrote: Triage: Are you still working on this @DesmondWillowbrook <https://github.com/DesmondWillowbrook> ? — Reply to this email directly, view it on GitHub <#94878 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEV3NIDLYL53VBOR4IZID4DYYR34BAVCNFSM5QRZQA4KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBQGIYDIOBYHE4A> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[kartva]

@rustbot release-assignment