Temporary allocators and the core library

Explain
1

I’ve been using Odin over the last year on and off with usually small programs. Overall I think it’s probably the closest thing to a C/C++ replacement for me but there is one thing that prevents me from switching entirely. I’ll describe it in a second but the purpose of this thread is to see if I’m just paranoid or if are these actual problems.

So, my biggest problem is with the implicit context, the temporary allocator, and its usage within libraries and in particular the core libraries. Some allocations are guarded by runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD() but it feels hacky because it assumes the default temporary allocator and when that’s not the case, the guard is “ignored” and the memory is “leaked”. Some allocations are matched with a free (like with dynlib > initialize_symbols > symbol_address > _symbol_address), but if we’re using the default temp allocator, this free operation is essentially a no-op so the memory is “leaked” again. Some allocations aren’t freed (like with os > read > win32.utf8_to_wstring on windows) which I’m not sure is worse but the memory is also “leaked”.

Now, I’m using “leaked” here but only until the temporary allocator is reset so it isn’t a permanent problem if the program is aware. The thing is that most of these allocations shouldn’t leave their scopes which makes me wonder whether something better could be done.

I’ve read some of the code in core:os/os2 and it seems it’s (at least partially) being addressed there but I’d argue it is not a localized problem. Probably the root of this is trying to fit multiple types of allocators into a common allocator interface.

I came up with a few solutions while writing this to not be too hand wavy but it’s not clear whether any would be a better fit. Either way:

1 - Allocator_Mode.Temp_Begin and Allocator_Mode.Temp_End
Similar to having Allocator_Mode.Free_All. It is not implemented for the default heap allocator (nor would malloc/free implementations allow it, I think) but is implemented for arena allocators, including the default temp allocator.

2 - Have temp_allocator be a different type (i.e. TempAllocator)
This would actually break any code mixing temporary and permanent/managed allocator but would have better separation of concerns (probably the worst one).

3 - Not have a temp allocator
This is probably the most controversial but let the application have its own temp/frame allocators like in core:os/os2.

Anyway, I’m sorry if this is confusing but it’s something that’s been bugging me out since forever and I wanted to hear from others. Over the last couple years, I’ve been looking for something to replace C/C++ with and currently the only real alternative is Odin. Jai could probably be another one but I just couldn’t get into the closed beta so I can’t tell. Zig could be but is too pedantic and honestly… a << @as(u5, b). Others are either GCd (Go, V), unreadable (Hare, Haskell), crazy (Rust). So it is either C/C++ or Odin and I’m really liking Odin.

This is already too long so I’ll just cut it here. Cheers.

2 Likes
2

We’re removing the runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD usage when os2 is ready. This is just a bodge effectively.

We are not adding any new allocator modes for this.

So that’s fundamentally all the problem is: they are bodges that will be removed entirely.

3

What you think would be the policy on using the temporary allocator after this cleanup? I’m asking because my preference would be to context.temp_allocator = panic_allocator at startup and manage any temporary memory separately (as with option #3 on the initial post). I think temporary allocators depend on the overall context of a codebase and while I see them being handy when used within it, I also see them being problematic when used outside the codebase boundaries, where the context could be different.

An example is the current use of runtime.DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD(). Even if it’s just bodge, it has a clear meaning inside the context of the core library which is to have some short lived memory within some scope but the problem arises because we can’t guarantee we’re using the default temporary allocator (the context of the program calling into the core library could be different).

I’ve mentioned some problems being addressed with os2 and I’m talking exactly about how it handles it’s own temporary memory with TEMP_ALLOCATOR_GUARD() and temp_allocator() rather than using context.temp_allocator and I’d still argue this is a better approach when interfacing with libraries.

Either way, I could just be overly paranoid or missing some detail. Cheers.

PS: I was thinking of writing a PR to change _load_library and _load_symbol in core:dynlib to avoid allocating at all when formatting strings. Is there a policy on using intrinsics.alloca? I could also just use a stack buffer with each system’s limits for symbol name and library path.

4

It is interesting that you mention in your point 2 that breaking code mixing allocator interfaces / types is a problem. DELETED

EDIT: My original post doesn’t make a whole tonne of sense, now that I have realised that there was actually a bug in my code and the temporary allocator wasn’t the issue. So I’ve removed most of it.

5

As I previously admitted, my initial repulsion towards the temporary allocator was – entirely – down to my own mistake. [1] However, having spouted about it, here, and subsequently deleted my post, I felt that I owed it to this thread to do some proper research and reading and make a better contribution.

As far as os / os2 are concerned, I feel that the problem is already less severe because the deprecation of os is well known or easily discoverable and os2 actually works for many things. In my case, I could move everything that was using the temporary allocator[2] to an os2 version (mostly file I/O) and work around the hidden dependency on temporary allocations, there.

But the problem is more wide-spread. Consider core:log which will also use temp_allocator and DEFAULT_TEMP_ALLOCATOR_TEMP_GUARD et cetera and that is in spite of the fact that initialising a log with something like create_console_logger() allows you to specify an allocator.

I can only assume that core:os and core:log aren’t the only places that use it. This means that I cannot look upon context.temp_allocator and think: “Ok… alright, fine… not my style but I’ll just not use it. Turn it off – others might enjoy it. Cool that they have that choice!” [3]

Now I don’t think that this is a show-stopper or something that needs to be resolved with any kind of urgency but I really would like the fate of the ambient allocators and particularly temp_allocator to be clarified.

Is the intended goal that one could somehow disable temp_allocator and, as long as they don’t then use it themselves, Odin still “just works”?

  • I would make that the default because one shouldn’t use it unless one intermittently frees what it allocates or else one has a leak that they didn’t write themselves – even if enabling it is as simple as a compiler switch to turn on the default.

  1. I had forgotten to pass my local allocator to one of my many delete() calls which had defaulted the argument to context.allocator which, of course, was a mismatch and thus a bad free. For a while, I was using temp_allocator as my local one and reading the documentation suggests that you “can’t” free on it (obviously!) and I had mistakenly assumed that the odd errors were it enforcing that, at run-time, instead of simply doing nothing. ↩︎

  2. Even os.exit() has a working variant in os2, I see. It surely doesn’t need temp_allocator but it is nice to know that in at least one .odin file, I no longer import core:os at all! ↩︎

  3. And there’s a lot to know about this, too. I still feel that I’m not understanding something, here. When I assigned context.temp_allocator to the panic allocator, my calls to log broke – leading me to find code in core:log that was using temp_allocator – but, elsewhere, I have some "c" calling-convention procs that begin by restoring context to my global one so that I can call other Odin-convention ones that use it and they seemed to be able to log just fine. There’s magic at work, there, I feel. ↩︎

6

Almost, the intended goal is that if you don’t use it or want to use it yourself, you shouldn’t be impacted by it or have to worry about it. This is currently already the case afaik.

So like you wanted, you can look at the context.temp_allocator and think not your style and not use it. Even with core:log, because it uses the guard procedure it cleans up after itself.

And while the DEFAULT_TEMP_GUARD works to meet that mentioned goal, any use of that is going to be removed and refactored out, and ultimately that guard will be removed.

7

Aha. Ok. I didn’t intuit that. But, now that you’ve clarified, my worries are mollified.

That’s enough to satisfy me, really. Even if it isn’t quite the reality, today, as long as I can conclude that any runtime impact of the temporary allocator is not expected if I explicitly avoid using it, I’m not too worried – it means that there will come a day where observing such a scenario could be justified as a bug and not ideal operation.

Of course, it’s very early days for me, with Odin, and I have no reason to expect bugs (now that you’ve clarified these things) so such an eventuality is extremely hypothetical. Thanks.