Add `Engine::serialization_compatibility_hash`

[lann]

Feature

Add a new Engine::serialization_compatibility_hash method that returns a hash of the "compiler info" appended to precompiled Wasm binaries.

Benefit

This would allow precompiled Wasm from multiple configurations/versions of Wasmtime to be safely and efficiently stored together by, for example, keying its storage on (Hash(wasm), serialization_compat_hash). This may still be a little pessimistic compared with Wasmtime's actual compatibility checks but would be better than the conservative approach of only reusing precompiled binaries on a single host.

Implementation

Extract the following section from append_compiler_info and reuse it to compute a sha-256 digest of the "compiler info":

wasmtime/crates/wasmtime/src/engine/serialization.rs

Lines 50 to 64 in c3c16eb

	let mut data = Vec::new();
	data.push(VERSION);
	let version = match &engine.config().module_version {
	ModuleVersionStrategy::WasmtimeVersion => env!("CARGO_PKG_VERSION"),
	ModuleVersionStrategy::Custom(c) => c,
	ModuleVersionStrategy::None => "",
	};
	// This precondition is checked in Config::module_version:
	assert!(
	version.len() < 256,
	"package version must be less than 256 bytes"
	);
	data.push(version.len() as u8);
	data.extend_from_slice(version.as_bytes());
	bincode::serialize_into(&mut data, &Metadata::new(engine)).unwrap();

Alternatives

• There is some overlap between the features this would be used for and Wasmtime's cache system, but the cache system is currently somewhat opaque and difficult to reason about wrt shared storage and the cache "worker"'s performance characteristics.
• This could be done today without changes to Wasmtime by precompiling a dummy module and hashing its .wasmtime.engine section, which seems fragile.
• Cranelift: split out a "settings" / flags infrastructure that works without the compiler proper #3900 suggests splitting out compatibility data into a separate structure, which appears to require a larger effort.

[alexcrichton]

One alternative idea for this would be to implement Hash and Eq for the Config type perhaps. That's more-or-less what the blob here is but would be a bit less specific to the current serialization strategy.

[lann]

implement Hash and Eq for the Config

That seems like it could work, though it would be a little more pessimistic compatibility-wise for settings that don't affect compilation or if a rustc update changes Hash (rare I assume).

One notable oddity is that we'd need to special-case Config::module_version's WasmtimeVersion to mix in CARGO_PKG_VERSION somewhere.

Would this be documented behavior of impl Hash for Config? Is it reasonable to assume that Config will continue to cover everything used in compatibility checks?

[alexcrichton]

That's true, yeah. I'm mostly thinking that I'd prefee to not bake in sha256 or something like that but to expose this through Hash and Eq if possible (in case anyone wants strict-equality as well instead of just hash-equality). One possibility would be something like Engine::cache_key(&self) -> impl Hash + Eq instead of returning the sha256 hash itself as well. We could then change the serialization.rs to use this directly to ensure nothing regresses as well perhaps.

[lann]

The semantics of Hash just don't seem quite right for this. You can get a u64 out of it for indexing but you wouldn't really use this key in a HashMap.

What about Engine::cache_key(&self) -> impl AsRef<[u8]>?

[alexcrichton]

I suppose that's probably the only reasonable thing to implement in the short term. I'm hesitant to have something that feels so specific here, but if it solves a use case for y'all and it's already used elsewhere it shouldn't really increase maintenance burden at all.

[lann]

Yeah, I actually think #3900 (if exposed publicly) would be a more generally-useful approach but I don't think I have the context/time to tackle it myself. I'm not actually opposed to the "build a dummy module and read out the compiler info" hack as a short term workaround but I wouldn't want to rely on it for very long.

[alexcrichton]

Nah it's much easier to "just" add a method to Engine to expose that section rather than harvesting it yourself, so I think that's ok to add.