reicast-emulator
reicast-emulator copied to clipboard
skmp
Restructure the codebase to not use globals
Currently, the codebase uses globals for any kind of state. While this is easy to read and fast for armv7/x86, it gets more of a pain with aarch64/x64, and also makes testing much more involved
Redream used the 'c-with-context-param' approach to solve this. Doing this makes initialization much more explicit, and thus, verbose. There is a bit of overhead to pay, though likely to not matter enough perf wise.
I'm not 100% convinced we should go this way, but it sounds like a good idea. I'm also not sure that using C structs w/ functions is better than C++ classes. Thoughts?
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
Inlining low-level performance critical functions will definitely help there. Otherwise, I expect the overhead will be negligible. classes vs structs is (for me) a matter of preference with a few caveats: Classes do force you to include other headers in your class header file when you use composition instead of aggregation. In C you can avoid this by using a pimple idiom, keep the definition in your .c/.cpp file and client code never needs to know what your structs are composed of. Needless to say, this will also help with compile times.
Can you please assign this task to me? I'm looking to refactor the codebase to get rid of global vars.
@psy-fidelious for your knowledge, pimpl idiom is successfully used in C++ too. via reference to a class member or unique_ptr. @luserx0 my idea is to put everything in classes through composition. so we don't rely on externs which are placed in random places of code. as much as I hate oop and love functional programming, organizing everything under classes seems like a nice idea to me.
It's not a bad idea, but it does nothing for us at this time .. The entire emulator needs an overhaul and refactor, why just wrap globals and still use mostly C-style code everywhere else? Unless we do the rewrite, I see no real benefit. We'd just end up w. a global Globals.var or .getVar(). , or have to pass a context, or make a singleton and query it to get its instance and do Globals::instance()->var ^ ...
I'm always open to suggestions, but again i see little real improment here except to say: Well now we don't have global vars because they are bad mmmkay?
@dmiller423 makes a very valid point. Partial rewrites just to disguise the core issue are something you do to look busy.
@pold500 the difference is that in C you can hide the contents of the struct itself in a C file, only exposing the interface in the header (struct declaration + functions operating on struct pointer). In C++ if one wants to expose the interface (methods) they need to also expose the contents of the class, which means include headers for any contained classes. You can choose to reference or allocate those on the heap, but that's not always desirable. Not that it's really important for this thread anyway, signing off.
Added to the Mid Term Goals milestone. @pold500 if you have ideas for this, can you write a small proposal here and/or make a small diff / proof of concept to illustrate how things would look?
@psy-fidelious Please read some article about pimpl idiom in C++. Short example, in C++:
class PimplClass;
class ClassUsingPimplClass
{
....
private:
PimplClass& pimplClass; //you don't include any headers for PimplClass.
//or with the unique ptr:
std::unique_ptr<PimplClass> m_pImpl;
}
@skmp ok, I'll make an experimental pr.
@pold500 forgive my lack of knowledge I will promptly go and educate myself on the matter further.
@psy-fidelious It's OK. My apologies for my selfish and entitled tone. I rephrased my comment.
I'm taking over this now as no progress has been made.
See #1760 #1764 #1765 #1766 #1769 #1660 and others.
@pold500 are you still willing to lend a hand?