Ensure that hashing across account data and resize area works #3889
Conversation
|
The Firedancer team maintains a line-for-line reimplementation of the |
seanyoung
force-pushed
the
hashers
branch
2 times, most recently
from
d5f28a1
to
0434fc2
Compare
| for item in chunk_iter { | ||
| let (region, src_addr, chunk_len) = item?; | ||
|
|
||
| let addr = Result::from(region.vm_to_host(src_addr, chunk_len as u64))?; |
There was a problem hiding this comment.
BTW, why do we do the translation at the callsite and not have it in the iterator?
There was a problem hiding this comment.
In fn iter_memory_pair_chunks() we may want to iterate in reverse, in which case we want to use the last address in the chunk rather than the first.
We could have an fn iter_memory_chunks() which does this.
| n_bytes: u64, | ||
| accounts: &[SerializedAccountMetadata], | ||
| memory_mapping: &MemoryMapping, | ||
| mut fun: F, |
There was a problem hiding this comment.
This could be a &mut dyn FnMut, instead of a generic.
There was a problem hiding this comment.
Do we really want &mut dyn FnMut?
- It's such a short function, code bloat shouldn't be an issue (I think)
fn iter_memory_pair_chunks()is also generic (so then both should be changed)
generic is "better inlining and optimization, but larger code" and dyn is "shorter slower code, indirect calling is expensive and fewer optimizations possible".
wdty?
Problem
The account data and resize area are separate regions with direct mapping. So, ensure that hashing data across it works.