Post a Comment On: cbloom rants

In code design, a frequent pattern is that of singleton state machines. eg. a module like "the log" or "memory allocation" which has various attributes you set up that affect its operation, and then subsequent calls are affected by those attributes. eg. things like : Log_SetOutputFile( FILE * f ); then Log_Printf( const char * fmt .... ); or : malloc_setminimumalignment( 16 ); then malloc( size_t size ); The goal of this kind of design is to make the common use API minimal, and have a place to store the settings (in the singleton) so they don't have to be passed in all the time. So, eg. Log_Printf() doesn't have to pass in all the options associated with logging, they are stored in global state. I propose that global state like this is the classic mistake of improving the easy case. For small code bases with only one programmer, they are mostly okay. But in large code bases, with multi-threading, with chunks of code written independently and then combined, they are a disaster. Let's look at the problems : 1. Multi-threading. This is an obvious disaster and pretty much a nail in the coffin for global state. Say you have some code like : pcb * previous_callback = malloc_setfailcallback( my_malloc_fail_callback ); void * ptr = malloc( big_size ); malloc_setfailcallback( previous_callback ); this is okay single threaded, but if other threads are using malloc, you just set the "failcallback" for them as well during that span. You've created a nasty race. And of course you have no idea whether the failcallback that you wanted is actually set when you call malloc because someone else might change it on another thread. Now, an obvious solution is to make the state thread-local. That fixed the above snippet, but some times you want to change the state so that other threads are affected. So now you have to have thread-local versions and global versions of everything. This is a viable, but messy, solution. The full solution is : There's a global version of all state variables. There are also thread-local copies of all the global state. The thread-local copies have a special value that means "inherit from global state". The initial value of all the thread-local state should be "inherit". All state-setting APIs must have a flag for whether they should set the global state or the thread-local state. Scoped thread-local state changes (such as the above example) need to restore the thread-local state to "inherit". This can be made to work (I'm using for the Log system in Oodle at the moment) but it really is a very large conceptual burden on the client code and I don't recommend it. There's another way that these global-state singletons are horrible for multi-threading, and that's that they create dependencies between threads that are not obvious or intentional. A little utility function that just calls some simple functions picks up these ties to shared variables and needs synchronization protection with the global state. This is related to : 2. Non-local effects. The global state makes the functions that use it non-"pure" in a very hidden way. It means that innocuous functions can break code that's very far away from it in hidden ways. One of the classic disasters of global state is the x87 (FPU) control word. Say you have a function like : void func1() { set x87 CW do a bunch of math that relies on that CW func2(); do more math that relies on CW restore CW } Even without threading problems (the x87 CW is thread-local under any normal OS), this code has nasty non-local effects. Some branch of code way out in func2() might rely on the CW being in a certain state, or it might change the CW and that breaks func1(). You don't want to be able to break code very far away from you in a hidden way, which is what all global state does. Particularly in the multi-threaded world, you want to be able to detect pure functions at a glance, or if a function is not pure, you need to be able to see what it depends on. 3. Undocumented and un-asserted requirements. Any code base with global state is just full of bugs waiting to happen. Any 3d graphics programmer knows about the nightmare of the GPU state machine. To actually write robust GPU code, you have to check every single render state at the start of the function to ensure that it is set up the way you expect. Good code always expresses (and checks) its requirements, and global state makes that very hard. This is a big problem even in a single-source code base, but even worse with multiple programmers, and a total disaster when trying to copy-paste code between different products. Even something like taking a function that's called in one spot in the code and calling it in another spot can be a hidden bug if it relied on some global state that was set up in just the right way in that original spot. That's terrible, as much as possible functions should be self-contained and work the same no matter where they are called. It's sort of like "movement of call site invariance symmetry" ; the action of a function should be determined only by its arguments (as much as possible) and any memory locations that it reads should be as clearly documented as possible. 4. Code sharing. I believe that global state is part of what makes C code so hard to share. If you take a code snippet that relies on some specific global state out of its content and paste it somewhere else, it no longer works. Part of the problem is that nobody documents or checks that the global state they need is set. But a bigger issue is : If you take two chunks of code that work independently and just link them together, they might no longer work. If they share some global state, either intentionally or accidentally, and set it up differently, suddenly they are stomping on each other and breaking each other. Obviously this occurs with anything in stdlib, or on the processor, or in the OS (for example there are lots of per-Process settings in Windows; eg. if you take some libraries that want a different time period, or process priority class, or priviledge level, etc. etc. you can break them just by putting them together). Ideally this really should not be so. You should be able to link together separate libs and they should not break each other. Global state is very bad. Okay, so we hate global state and want to avoid it. What can we do? I don't really have the answer to this because I've only recently come to this conclusion and don't have years of experience, which is what it takes to really make a good decision. One option is the thread-local global state with inheritance and overrides as sketched above. There are some nice things about the thread-local-inherits-global method. One is that you do still have global state, so you can change the options somewhere and it affects all users. (eg. if you hit 'L' to toggle logging that can change the global state, and any thread or scope that hasn't explicitly sets it picks up the global option immediately). Other solutions : 1. Pass in everything : When it's reasonable to do so, try to pass in the options rather than setting them on a singleton. This may make the client code uglier and longer to type at first, but is better down the road. eg. rather than malloc_set_alignment( 16 ); malloc( size ); you would do : malloc_aligned( size , 16 ); One change I've made to Oodle is taking state out of the async systems and putting in the args for each launch. It used to be like : OodleWork_SetKickImmediate( OodleKickImmediate_No ); OodleWork_SetPriority( OodlePriority_High ); OodleWork_Run( job ); and now it's : OodleWork_Run( job , OodleKickImmediate_No, OodlePriority_High ); 2. An options struct rather than lots of args. I distinguish this from #3 because it's sort of a bridge between the two. In particular I think of an "options struct" as just plain values - it doesn't have to be cleaned up, it could be const or made with an initializer list. You just use this when the number of options is too large and if you frequently set up the options once and then use it many times. So eg. the above would be : OodleWorkOptions wopts = { OodleKickImmediate_No, OodlePriority_High }; OodleWork_Run( job , &wopts ); Now I should emphasize that we already have given ourselves great power and clarity. The options struct could just be global, and then you have the standard mess with that. You could have it in the TLS so you have per-thread options. And then you could locally override even the thread-local options in some scope. Subroutines should take OodleWorkOptions as a parameter so the caller can control how things inside are run, otherwise you lose the ability to affect child code which a global state system has. Note also that options structs are dangerous for maintenance because of the C default initializer value of 0 and the fact that there's no warning for partially assigned structs. You can fix this by either making 0 mean "default" for every value, or making 0 mean "invalid" (and assert) - do not have 0 be a valid value which is anything but default. Another option is to require a magic number in the last value of the struct; unfortunately this is only caught at runtime, not compile time, which makes it ugly for a library. Because of that it may be best to only expose Set() functions for the struct and make the initializer list inaccessible. The options struct can inherit values when its created; eg. it might fill any non-explicitly given values (eg. the 0 default) by inheriting from global options. As long as you never store options (you just make them on the stack), and each frame tick you get back to a root for all threads that has no options on the stack, then global options percolate out at least once a frame. (so for example the 'L' key to toggle logging will affect all threads on the next frame). 3. An initialized state object that you pass around. Rather than a global singleton for things like The Log or The Allocator, this idea is to completely remove the concept that there is only one of those. Instead, Log or Allocator is a struct that is passed in, and must be used to do those options. eg. like : void FunctionThatMightLogOrAllocate( Log * l, Allocator * a , int x , int y ) { if ( x ) { Log_Printf( l , "some stuff" ); } if ( y ) { void * p = malloc( a , 32 ); free( a , p ); } } now you can set options on your object, which may be a per-thread object or it might be global, or it might even be unique to the scope. This is very powerful, it lets you do things like make an "arena" allocator in a scope ; the arena is allocated from the parent allocator and passed to the child functions. eg : void MakeSuffixTrie( Allocator * a , U8 * buf, int bufSize ) { Allocator_Arena arena( a , bufSize * 4 ); MakeSuffixTrie_Sub( &arena, buf, bufSize ); } The idea is there's no global state, everything is passed down. At first the fact that you have to pass down a state pointer to use malloc seems like an excessive pain in the ass, but it has advantages. It makes it super clear in the signature of a function which subsystems it might use. You get no more surprises because you forgot that your Mat3::Invert function logs about degeneracy. It's unclear to me whether this would be too much of a burden in real world large code bases like games.