-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use u32 for log array indexing and length #42
Comments
The log array expects a 32-bit or greater architecture. This is just a compiler check to confirm that. Related to terminusdb#42.
Right, I have no intention of supporting smaller architectures. Much of rust's api dealing with slices or offsets or indexes expect an usize. Exposing len as an usize is therefore convenient. The alternative would be to cast whatever comes out of len in such situations. The problem with smaller architectures doesn't go away by moving the cast to the caller. Making logarray work on 16 bit would require some more effort. So I'm in favor of keeping this a usize. Your assert is a good idea and should let us get away with this. That said, does rust even support small architectures? |
I agree with not supporting anything less than 32-bit. Yes, I believe Rust does support (or would not be averse to supporting) some architectures with smaller bus sizes. There's a lot of movement toward making it work well in embedded systems. Having looked more at the data structures used in
I'm guessing 32-bit would be a rather minor thing to focus on, and so option 2 is probably the better way to go. If that's the case, just in case anybody who happened to try running |
The log array buffer format uses a
u32
in the control word to determine the length (number of elements) in the log array. But this is exposed as ausize
publicly for indexing and getting the length.I'm guessing you have no plans to develop
terminus-store
for an architecture in whichstd::mem::size_of::<usize>()
< 4 (i.e. the bus width is < 32 bits), so you won't have a problem withusize
being too small. But maybe another problem will appear due to casting or running on a 32-bit platform.It seems like it would be better to more accurately represent the index and length types. For one thing, given that a log array is read from a disk and (I think) should be portable across systems, it will never have a length determined by a particular system's architecture.
Unfortunately, changing
usize
tou32
in the appropriate places inlogarray.rs
introduces a cascade of errors around the code. I haven't looked into these. Maybe a more thorough audit of of data structure lengths and indexing is needed.This is possibly a minor issue, but I just wanted to make a note of it before I moved on and forgot about it.
The text was updated successfully, but these errors were encountered: