Handmade Hero » Forums » Code » Reasoning behind #defines for PushSize and friends
KanjiOwl
6 posts
#20619 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago Edited by KanjiOwl on March 2, 2019, 3:34 a.m. Reason: Initial post

On Day 88, Casey mentioned that he made PushSize a macro for "debugging reasons" (link). Unless I've missed it, I haven't seen where these #defines have been flexed.

He mentioned that he wanted everything hitting the allocator to go through a macro instead of a direct function call. In the past, I've seen this pattern used so you can easily inject your own custom allocators in place of the default one. Not sure if this is what he had in mind, or there is something more clever you can do. Is there some common debugging technique involving macros in this way that I'm unaware of?
mrmixer
Simon Anciaux
656 posts
#20620 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago

I believe it's to be able to change the parameter that are passed to PushSize_ without changing the calls, for example by using a define passed to the compiler. I don't remember if Casey's does that at some point. But it could be used to enable / disable overflow and underflow checking.


KanjiOwl
6 posts
#20626 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago

Hmm, in this case, overriding the macro to modify the parameters would be tough to get correct. Since the macros are variadic, the caller may or may not supply a params struct. You would need to do some fancy footwork to handle both cases.

If all you wanted to do was globally enable bounds checks, the way he ended up doing it (by or-ing in the flag inside PushSize_) seems like the easiest way to go about it. You could wrap that statement in an #ifdef, macroize the rvalue, and have yourself a party. For example,

1
2
3
4
5
#ifdef EXTRA_ALLOCATION_FLAGS
Arena->AllocationFlags |= EXTRA_ALLOCATION_FLAGS;
#endif
if (Arena->AllocationFlags & (PlatformMemory_OverflowCheck | PlatformMemory_UnderflowCheck)) {
...


Of course, the downside to this approach is you give up granular control over who gets these flags and who doesn't. Presumably, you macroized the callsites to give yourself the option of easily switching the checks on for a particular file or block of code (or for some other, unknown reason). The code above is an all-or-nothing deal.

Looking at it further, the allocation flags live outside the params structure (for whatever reason), so I think you could use something like the following if you wanted more control,
1
#define PushSize(Arena, Size, ...) (((Arena)->AllocationFlags |= <flags>), PushSize_(Arena, Size, ## __VA_ARGS__))


Maybe that is one reason to #define all your allocators?
mrmixer
Simon Anciaux
656 posts
#20627 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago Edited by Simon Anciaux on March 2, 2019, 6:09 p.m. Reason: typo

It was a bad idea to point to the video with the overflow/underflow checks.

What I meant was that if it's a macro you can easily swap it for a different function at compile time with a compiler flag and no cost at run time (no ifs to see which path to take). Something like:
1
2
3
4
5
6
7
#if defined UNDERFLOW_CHECK
#define PushSize(Arena, Size) PushSize_UnderflowCheck(Arena, Size)
#elif defined OVERFLOW_CHECK
#define PushSize(Arena, Size) PushSize_OverflowCheck(Arena, Size)
#else
#define PushSize(Arena, Size) PushSize_(Arena, Size)
#endif

In a sens it's like you said: an easy way to swap the allocator.
KanjiOwl
6 posts
#20628 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago Edited by KanjiOwl on March 2, 2019, 11:12 p.m. Reason: Code formatting

Thanks, that makes sense. It still feels like there is some untapped potential that's going unnoticed. Maybe I've missing the point, but in my mind, using macros for this reason doesn't buy you that much more convenience than say, changing the name of functions to achieve the same effect. For example,

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
void *PushSize_UnderflowCheck(...) { /* Check for underflows */ }
void *PushSize_OverflowCheck(...) { /* Check for overflows */ }
void *PushSize_SuperAwesome(...) { /* The awesome allocator */ }
void *PushSize_SuperAwesome_v2(...) { /* A better version of the awesome allocator */ }
void *PushSize_(...) { /* The default allocator */ }

// Sometime later you decide to swap the default allocator. Simply change a couple of symbol names.

void *PushSize_UnderflowCheck(...) { /* Check for underflows */ }
void *PushSize_OverflowCheck(...) { /* Check for overflows */ }
void *PushSize_(...) { /* The awesome allocator */ }
void *PushSize_SuperAwesome_v2(...) { /* A better version of the awesome allocator */ }
void *PushSize_Original(...) { /* The default allocator */ }


This also gets you zero overhead and involves changing 2 lines, compared to the 1 you would change for redefining the macro.

If you are a 3rd-party library, sure, it makes sense to provide macros to swap out the allocator so users don't have to reach into your code to do it. But if you are in control of the entire codebase, it seems like a fair amount of effort to push everything through macros for a slight convenience, unless you had some big wins in mind.
mrmixer
Simon Anciaux
656 posts
#20632 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago

The macro isn't used to "evolve the code", it's used to switch the allocator for a quick test.

If you have a build system/server, it could provide the ability to build and run the game with a different allocator to automatically test for memory overflow.

Also there is a difference in your example: the real allocator is not a 1 line comment, and changing the lines require a bit of "search for the right lines" which you need to do each time you change the allocator (2 times for a quick enable/disable test) and is a bit error prone. It's not much, but it's there.

Maybe we should ask Casey what he meant directly on stream, since most of the conversation is based on my interpretation.
KanjiOwl
6 posts
#20633 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago

I agree, the method I described doesn't lend itself to fast toggles, automated build support, and is also easier to mess up. It certainly has it's drawbacks, and I'm not trying to advocate any particular method over another.

My point was more towards suggesting that a more braindead approach gets you roughly 80% of the way there, and I was wondering if macros only bought you that final 20%, or was there something more that they brought to the table.

I appreciate your insights and perspective, which more or less confirm my initial understanding of the benefits. It would be interesting to hear Casey's rationale behind this. If I can catch a pre-stream/Q&A one of these days, I'll remember to ask.
marcc
Marc Costa
43 posts
#20637 Reasoning behind #defines for PushSize and friends
5 months, 3 weeks ago

Macros are also useful to add some extra information to function calls without burdening the caller, e.g. line number and file where the function is called from:

1
2
#define Allocate(size) Allocate_(size, __FILE__, __LINE__, __FUNCTION__)
void* Allocate_(umm size, const char* filename, int line, const char* function_name){...}
KanjiOwl
6 posts
#20665 Reasoning behind #defines for PushSize and friends
5 months, 2 weeks ago

This is true, I've seen many logging and assertion systems use/abuse this functionality to automatically inject various bits of data (filename, line number, status codes, etc.) I've worked on projects in the past where macros like these were relied on heavily to capture the runtime and interplay between components in a complex system. Inserting the information manually, particularly the line numbers, would be a disaster without macros.
rationalcoder
Blake Martin
31 posts
#20686 Reasoning behind #defines for PushSize and friends
5 months, 2 weeks ago Edited by Blake Martin on March 9, 2019, 5:48 a.m.

I doubt that changing the allocator has anything to do with it. It's almost certainly just there to have a convenient place to add file/line information. That way, when you exhaust an arena and need to debug it, you can inject some code that can show you a graphical display of memory arena allocations by call site. When you have that information, the culprit becomes obvious.

On the other hand, adding this information kind of implies what you are talking about, since you would need to swap out the current allocator for a debug one that takes file/line information, logs it, and then calls the default one.
KanjiOwl
6 posts
#20695 Reasoning behind #defines for PushSize and friends
5 months, 2 weeks ago

Pretty much nailed it on the head. Asked Casey about it in the Q&A on Day 517, and he basically said that it lets you track allocations by file/line if you need to.

Another benefit he mentioned was getting a little bit more type safety, with calls like PushStruct/PushArray expanding out to include a cast on the return value. That's something that can't easily be replaced with a standard function call.