Origen-SDK / o2

MIT License
4 stars 0 forks source link

Problem getting a ref to DUT #24

Closed ginty closed 4 years ago

ginty commented 4 years ago

Hi @coreyeng,

This isn't ready to merge, but I opened it for discussion with you.

So I started trying to implement dut.memory_maps which would return a Python class defined in Rust along the lines of how we have been discussing in #21

That all went fine initially and dut.db.memory_maps("some.path") returns the object defined from Rust - https://github.com/Origen-SDK/o2/blob/block_loader/rust/origen/pyapi/src/memory_map.rs#L12 At the time of executing that function to instantiate the object I have full access to the Rust DUT as planned.

However, I've now hit a roadblock.

My plan was to store a copy of the path to the model with the MemoryMaps struct and then use that to resolve the model any time I needed to get data from it.

However, I very quickly realized when trying to implement dut.db.memory_maps("some.path").len(), that when you get down to methods of the MemoryMaps struct itself, you no longer have access to the DUT!!! - https://github.com/Origen-SDK/o2/blob/block_loader/rust/origen/pyapi/src/memory_map.rs#L39

I think the only way around this problem is going to be to store some kind of magic reference to either the relevant model or the top-level DUT itself within the MemoryMaps struct (and all others like it).

But damned if I know how...

coreyeng commented 4 years ago

Yeah... this was one of those things that made me completely rethink how I'd program in Rust, though, it also made me understand what you were saying about not wanting to deal with memory issues and such. I was pretty annoyed for a while but then realized it genuinely is just Rust doing its job. It'd be easy in C++, but then someone else (or me by mistake) could go in wipe out the data structure by accident and then.. invalid memory accesses!

What I ended up in doing in pin collections was passing a reference to the container around to all the functions. Its a bit messy, but it gets us around having to give it a lifetime parameter, and from doing any of that Ref stuff, which I think we might be able to force, but it won't be very friendly. Could do the same: just pass the model itself to each and every function call...

That'll be cumbersome though, so I wonder if adding a static structure in lib.rs, along with the CONFIG and STATUS, to hold the DUT. This could be instantiated from the pyo3 side when it comes into scope, then we can have a function on that which will return Result<Model, Err>, where the Err can be if a DUT isn't instantiated from Python. We can also put the tester here, along with any other global things. This'll allow for static placeholders, so Rust should be happy, but then can go into and out of scope as needed. Rust should guarantee the thread-safety of this too since if a thread is borrowing it Rust won't let it go out of scope.

I think that'll be closest we can get to backtracking without passing the object itself to every function, or introducing lifetimes. I already tried a custom lifetime, but then found out pyo3 doesn't support them and the only way around that is it make it a static lifetime anyway.

coreyeng commented 4 years ago

I just stumbled on some pointer stuff that Rust does allow. Might be able to use this to store pointers to the DUT from the subblocks. Now we're getting back to the C++ world though.

ginty commented 4 years ago

Hi @coreyeng,

OK I played around with it a bit and changed it to make the DUT a (mutable) singleton in Rust rather than being part of the (Python memory owned) PyDUT class.

I think this works much better, I actually had something similar originally but I was lead astray by the groupthink on Stackoverflow about globals being bad. However, I think in this case it makes absolute sense and is the right solution.

It is behind a Mutex to avoid any future issues with multi-threading, so to get a read-only lock on the DUT do:

let dut = origen::DUT.lock().unwrap();

and for a mutable version it is just:

let mut dut = origen::DUT.lock().unwrap();

The result returned from lock() can only be an error if a thread crashes (panics) while holding a lock. The docs for the Mutex say that most people will just simply unwrap this and not handle such an error, just letting the subsequent panic occur in the remaining threads - i.e. something pretty bad and not really recoverable has already happened in this situation.

I've made the tests green and I would propose merging this if you are happy to so that you can use it too.

ginty commented 4 years ago

Also fixed and removed all the manual error conversion from origen::Error that was being done in PyApi