pineman / fpt

Gameboy emulator (yes, written in rust)
https://pineman.github.io/fpt
MIT License
5 stars 1 forks source link

move code listing to array #44

Closed pineman closed 3 months ago

pineman commented 3 months ago

Move the code listing to an array and clear certain regions of it when we know the code changes - bootrom unloading, rom switching, etc.

This assumes self modifying code is not a thing

@diogotito what do you think of this? we should probably go with your idea of not duplicating this code array. Our memory should be an array of structs that may contain the code string. I was thinking something like:

struct MemoryByte {
    byte: u8,
    code: Option<String>
}

pub struct Memory {
    mem: [MemoryByte; 65536],
    cartridge: Vec<u8>,
    bootrom: &'static [u8; 256]
}

WDYT?

diogotito commented 3 months ago

My first reaction is: I don't like the idea of the bytes in Memory being interspersed with metadata like this. It feels better to store code as a separate array, making this more of a struct of arrays.

pineman commented 3 months ago

sure! if this were C I'd know how much memory the code array is using, but I don't know enough rust yet to estimate how much an array of 2**16 Option is 😆 but the overall point of it being the same together in e.g. struct MemoryByte versus separated still stands, so I agree

so please review it as it is, i.e. keeping the separate implementation

diogotito commented 3 months ago

Actually, Option has some interesting optimisation guarantees that would apply if each line of code was a Box<str>.

With (u8, Option<String>) I think the layout would be

Box<str> could optimize away the discriminant and the capacity (an isize and an usize). Maybe the string length would also be stored outside the array? No, because str is a Dynamically Sized Type and Box would have to store a fat pointer

It seems wasteful compared to $2^{16}$ tightly packed u8s (are they?).

OTOH its not like we are benefiting much from having the Gameboy's bytes stored contiguously right now, so I'm not super sure on my SoA stance.