Optimize hashing by implementing is_hash_trivial - #7695
Conversation
PR SummaryHigh Risk Overview
Tests expand hash coverage (tuple arity, trivial classification, composed hashing) and every custom Reviewed by Cursor Bugbot for commit 514b41c. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c9e5c7f. Configure here.
|
👍 |
Performance improvementso2
|
| Improvements | Regressions | |
|---|---|---|
| Count | 35 | — |
| Average | 25.47% | — |
| Median | 24.53% | — |
| Max | 45.46% | — |
| Min | 12.58% | — |
Max matches benchmark improved for 37.62%.
o2 trade-account benchmarks
| Improvements | Regressions | |
|---|---|---|
| Count | 21 | — |
| Average | 7.93% | — |
| Median | 8.29% | — |
| Max | 11.53% | — |
| Min | 4.43% | — |
## Description This PR introduces a new `__mem_repr_eq` intrinsic for comparing memory representation of types, and removes the existing `__runtime_mem_id` and `__encoding_mem_id` intrinsics. We decided to introduce `__mem_repr_eq` to mitigate the following issues we had with existing intrinsics: - `mem_id`s were `u64` values computed using `DefaultHasher` whose results are not guaranteed to be stable across different runs of the same Sway compiler or being same on different target architectures. - even if we switched to a stable hasher, hashing to `u64` was not giving a strong no-collision guarantee. A hash collision between different, e.g. runtime and encoding representation, would in case of false positives result in wrong encoding or decoding. The `__mem_repr_eq` is defined as: ```sway __mem_repr_eq<T>(repr_a: str, repr_b: str) -> bool ``` It returns `true` if the memory representation `repr_a` of the type `T` is equal to its memory representation `repr_b`, assuming `T` has both memory representations. If `T` does not have any of the representations `repr_a` or `repr_b`, returns `false`. The valid values for `repr_a` and `repr_b` are `"runtime"`, `"encoding"`, and `"hashing"`: - `"runtime"` is how the type is represented inside the VM's memory. This is the Sway runtime memory representation (e.g., struct fields are aligned to word boundaries, arrays are packed, etc.). This memory representation is defined for every type. - `"encoding"` is the packed memory representation of a type, as defined by the canonical ABI encoding. Not all types have a canonical ABI encoding defined, e.g., dynamic types like `Vec` or `raw_slice`. In that case, `"encoding"` never compares equal to any other memory representation, __including to itself__. - `"hashing"` is the packed memory representation of a type, as defined by the canonical hashing introduced in #7695. Not all types have a canonical hashing defined, e.g., dynamic types like `Vec` or `raw_slice`. In that case, `"hashing"` never compares equal to any other memory representation, __including to itself__. To test if a type `T` has `"encoding"` or `"hashing"` memory representation defined, compare that representation to itself. E.g.: `let has_encoding_repr = __mem_repr_eq<T>("encoding", "encoding");` `repr_a` and `repr_b` must be compile-time constant `str`s, whose values are one of `"runtime"`, `"encoding"`, or `"hashing"`. The constant `str`s never end up in the bytecode. Additionally, the PR moves existing E2E intrinsics tests that were not in the `language/intrinsics` into `language/intrinsics`. ## Breaking Change The old `__runtime_mem_id` and `__encoding_mem_id` intrinsics are removed. The code that is using them must switch to the new `__mem_repr_eq`. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.

Description
This PR extends the
Hashtrait with theis_hash_trivial() -> boolassociated function. This function is supposed to return true if the hash byte representation of the type is the same as its runtime memory representation.By using
is_hash_trivial,std::hash::sha256andkeccak256module functions can skip expensiveBytescreation in theHasherand directly hash the value's memory content.The performance gains on o2 benchmarks are significant. In average, 25.47% improvements on
order-bookand 7.93% improvements ontrade-account. Max matches benchmark improved for 37.62%. (Details are given in the comment below.)This optimization is similar to what
is_encode/decode_trivial()associated functions are providing for ABI encoding/decoding.Similar to ABI encoding/decoding, implementation of hashing for a type is in full control of the Sway package that owns the type. In practice, and in
stdin particular, non-dynamic types are in general trivially hashable if their packed memory representation is the same as their runtime memory representation.Because of this, when implementing
Hashfor tuples, we use the__runtime_mem_idand the__encoding_mem_idintrinsics. While using the__encoding_mem_idmight look like coupling encoding and hashing, it is not. What we are actually using is the notion of the "packed memory representation" that the__encoding_mem_idprovides. Unfortunately, the intrinsic that provides this information was originally created for ABI encoding and not for a more generic usage, e.g.,__packed__mem_id.To make the distinction clear and remove what looks like coupling of unrelated concepts, we will provide a follow-up PR that introduces intrinsics for use case based memory layouts:
__mem_repr_id_runtime__mem_repr_id_encoding__mem_repr_id_hashingCurrently, all enums in the
stdhash their descriminators asu8. This makes them all being non-trivially hashable. For many of them, hashing the descriminator asu64would make them trivially hashable. This is however a breaking change that potentially affects the hashed data we already have deployed, e.g.,StorageMapkeys.In the future, we can provide this breaking change behind a feature flag. Enums that can benefit from being trivially hashed are marked in code with
TODO: (HASH-TRIVIAL-ENUMS).Checklist
Breaking*orNew Featurelabels where relevant.