lann edited PR #5826 from precompile-compat-key
to main
:
Draft for #5802 for naming / impl feedback prior to writing tests.
After writing the implementation I landed on
precompile_compatibility_key
to show the relationship withprecompile_{module,component}
, but can revert to the discussedcache_key
if desired.
lann edited PR #5826 from precompile-compat-key
to main
:
Draft for #5802 for naming / impl feedback while working on tests.
After writing the implementation I landed on
precompile_compatibility_key
to show the relationship withprecompile_{module,component}
, but can revert to the discussedcache_key
if desired.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Hashbrown uses ahash which doesn't guarantee stability between different systems. In fact it returns different values between systems with hardware AES instructions and those that don't. As such using ahash here will break cross-compilation.
lann submitted PR review.
lann created PR review comment:
Ah good to know, thanks.
lann updated PR #5826 from precompile-compat-key
to main
.
lann submitted PR review.
lann created PR review comment:
Replaced with sha2, which is probably where I should have started anyway. Its a new dep under some feature sets but it is already used by the cache system.
lann edited PR review comment.
bjorn3 submitted PR review.
bjorn3 created PR review comment:
sha512 is faster on 64bit systems as it can consume more bytes at once. Not that it really matters here. This hashing is highly unlikely to ever become a bottleneck.
alexcrichton submitted PR review.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I think the documentation here may want to be a bit more nuanced because if two blobs are the same then you should be able to load between engines but if they're different they may still load correctly. For example different values of
generate_address_map
will produce different blobs but that's not checked when loading modules into engines.I think it may be best to reword this as something along the lines of "if two blobs are equal artifacts from one engine are guaranteed to be loadable in another" or something like that.
alexcrichton created PR review comment:
I think it's fine to just return the whole blob here and let embedders hash it if they want. That avoids a dependency here and should enable a bit more flexibility for embedders as well.
alexcrichton created PR review comment:
If this compiles without the
#[cfg]
no need for the attribute. If the#[cfg]
is required, though, then this attribute is also required for better display in documentation.
lann submitted PR review.
lann created PR review comment:
Yeah that's what I was trying to convey but "if" is ambiguous. I'll be more explicit about it.
lann updated PR #5826 from precompile-compat-key
to main
.
lann updated PR #5826.
Last updated: Dec 23 2024 at 13:07 UTC