Why shouldn't `context` inside a procedure require an explicit procedure parameter?

[hintron]

I know this is a skill issue on my part, but let me explain what I got tripped up on first, and then ask a dumb question about an Odin language design decision.

For wasm builds, you can’t have a single main() procedure - instead, you have to break things out into init(), step(), and shutdown(). So that’s what I did, and I added code in init() to set the context allocator to a tracking allocator (following the Odin book), like so:

init :: proc() {
	when ODIN_DEBUG {
		mem.tracking_allocator_init(&mem_tracker, context.allocator)
		context.allocator = mem.tracking_allocator(&mem_tracker)
	}
	...
}
step :: proc() -> bool {
	...
}
shutdown :: proc() {
	...
	when ODIN_DEBUG {
		if len(mem_tracker.allocation_map) > 0 {
			for _, entry in mem_tracker.allocation_map {
				fmt.eprintf("%v leaked %v bytes\n", entry.location, entry.size)
			}
		}
		mem.tracking_allocator_destroy(&mem_tracker)
	}
}

However, I was seeing memory leaks that I did not expect. Karl helpfully pointed out that I was setting up the tracking allocator wrong - that the context I was setting was actually just for the scope of the init() procedure. (See Issues · karl-zylinski/karl2d · GitHub)

As a noob to the language, I was assuming that I was setting the global context (and that’s what it feels like to me, coming from nearly any other language). I now understand, though, that when you are inside a procedure, context refers to the context passed into that procedure, and in the global scope, context is the global context.

So my language design question is this: Why shouldn’t context inside a procedure require an explicit procedure parameter? E.g. something like this:

init :: proc(ctx: contextT) {
	when ODIN_DEBUG {
		mem.tracking_allocator_init(&mem_tracker, ctx.allocator)
		ctx.allocator = mem.tracking_allocator(&mem_tracker)
	}
	...
}

(I’m not sure what type ctx should be, but I’m guessing it would be type aliased to something)

Some advantages to this approach:

1. Procedures now declare that they are modifying the context in the procedure signature, so it’s easier to see when context shenanigans are going on
2. It is easier for newcomers to the language, since context now would not look like you are setting a global variable
3. You can set the global context from within a procedure instead of doing the thing where you have to save it to a variable first.

And since setting the global context from within a procedure is rare, maybe it could be gated behind some kind of #directive so that newcomers don’t unwittingly override the global context when they forget to add the procedure parameter.

Those are just my initial thoughts after hitting this issue for the first time. I’m no language expert, so I am happy to hear why this wouldn’t be a good idea, or if this has some merit. Thanks

[xuul]

I have zero experience with wasm, but Karl’s comment about scope makes sense. If the tracking allocator is set at the beginning of main in a standard program, and since the scope of main encapsulates everything that comes afterward (with minor exceptions), the context is passed through to the end because of the inherent nature of the “odin” calling convention. Since wasm, as mentioned, does not use main, but rather calls individual procedures, (init, shutdown, etc), the scope of the tracking allocator starts and ends with each procedure since they are not calling the other in a chain or at least do not originate from the same scope (i.e. main). If they did, the standard “odin” calling convention, which implicitly passes the context by pointer behind the scenes, would be passed along.

Have you tried explicit-context-definition? Not sure if that would work, but that’s where I would start.

Reading odins-most-misunderstood-feature-context may also help shed some light.

(I’m not sure what type ctx should be, but I’m guessing it would be type aliased to something)

If there is a need to explicitly pass the context in a parameter, it would be defined as runtime.Context. Then at the beginning of the procedure that receives the parameter, set it to “context”.

proc_wants_context :: proc(ctx: runtime.Context) {
	context = ctx
	// do stuff
}

//pass with "context" keyword
proc_wants_context(context)

// or explicit context definition
i_get_my_own_context :: proc() {
	context = runtime.default_context()
	//do stuff
}

[GigaGrunch]

Just for completeness, here is the documentation of the implicit context system: Overview | Odin Programming Language

[mechodin]

It’s one (not the) way to set up the tracking allocator.

It’s easier to understand the context variable if you look up the word definition of context, which doesn’t imply being ‘global’.

@hintron

Procedures now declare that they are modifying the context in the procedure signature, so it’s easier to see when context shenanigans are going on

 my_proc :: proc(x: int) {...} 

What does it do to the x variable? Is this a write or a read operation?

context is copy-on-write. If your procedure tries to modify the outer scope’s context,
it gets a copy of that context, not a pointer to it,
which means your procedure won’t affect the outer scope’s context.

[GigaGrunch]

To make it a bit clearer what implicitly passed context means, I created a little example in C:

#import <stdio.h>

typedef struct {
	int user_index;
} Context;

void fun_1(Context context);
void fun_2(Context context);

int main() {
	Context context = {};
	context.user_index += 1;
	printf("[main]  user_index: %i\n", context.user_index);
	fun_1(context);
	printf("[main]  user_index: %i\n", context.user_index);
}

void fun_1(Context context) {
	context.user_index += 1;
	printf("  [fun_1] user_index: %i\n", context.user_index);
	fun_2(context);
	printf("  [fun_1] user_index: %i\n", context.user_index);
}

void fun_2(Context context) {
	context.user_index += 1;
	printf("    [fun_2] user_index: %i\n", context.user_index);
}

Here is the same code in Odin. Note that the context is always passed implicitly because every procedure uses the default calling convention:

package test

import "core:fmt"

main :: proc() {
	context.user_index += 1
	fmt.printfln("[main]  user_index: %v", context.user_index)
	fun_1()
	fmt.printfln("[main]  user_index: %v", context.user_index)
}

fun_1 :: proc() {
	context.user_index += 1
	fmt.printfln("  [fun_1] user_index: %v", context.user_index)
	fun_2()
	fmt.printfln("  [fun_1] user_index: %v", context.user_index)
}

fun_2 :: proc() {
	context.user_index += 1
	fmt.printfln("    [fun_2] user_index: %v", context.user_index)
}

Output for both versions is exactly the same:

[main]  user_index: 1
  [fun_1] user_index: 2
    [fun_2] user_index: 3
  [fun_1] user_index: 2
[main]  user_index: 1

[hintron]

Ok, that all makes sense. But just to clarify:

context is passed into a procedure by pointer, not value, right? However, because of the “copy on write” property, it acts as if we pass it by value. So if I were to modify your C code to show what actually happens in reality, then shouldn’t it look like the following? Instead of this:

void fun_1(Context context) {
	context.user_index += 1;
	printf("  [fun_1] user_index: %i\n", context.user_index);
	fun_2(context);
	printf("  [fun_1] user_index: %i\n", context.user_index);
}

it would really be this:

void fun_1(Context *context) {
	Context context_copy = *context; // Copy on write
	context_copy.user_index += 1;
	printf("  [fun_1] user_index: %i\n", context_copy.user_index);
	fun_2(&context_copy);
	printf("  [fun_1] user_index: %i\n", context_copy.user_index);
}

(assuming fun_2() has a similar change)

Also, another thing - I’m assuming this copy on write behavior is just a shallow copy, because I don’t see how Odin would know how to do a deep copy if you e.g. have a pointer in user_ptr. So if you modify whatever context.user_ptr points to, that’s an example of a change that won’t be “reverted” when you exit the scope, right?

[mechodin]

You’re mixing things up.
The C example was given to explain the meaning of the implicitly passed context.

Now you’ve moving from the context itself to its user_ptr’s underlying data.

If the following snippet is what you mean,

package main

import "core:fmt"

modify_user_ptr_data :: proc(px: ^int) {
	px^ = 5
}

main :: proc() {
	x := 100
	context.user_ptr = &x
	fmt.println((cast(^int)context.user_ptr)^)

	modify_user_ptr_data(&x)
	fmt.println((cast(^int)context.user_ptr)^)
}

then you’re changing the data, but only because you’ve been given access to the data from the outer scope.

[xuul]

Maybe it will be helpful to see how it is possible to setup a tracker independent of main(). Bare with me though, I’m not familiar with wasm, so I’ve made an assumption that it is possible to define a global that stays initialized until shutdown procedure is called. If that is not true, then disregard the below and learn me on how globals work in wasm.

2 main things to point out

1. tracking_allocator is intialized as a global
2. Every procedure needs to set context.allocator when ODIN_DEBUG for the tracker to capture the data

I tested this using standard main procedure, but that was only for me to prove out the concept. step() allocates memory, but does not delete it.

main :: proc() {
	init()
	step()
	shutdown()
}

// Global tracker
tracking_allocator: mem.Tracking_Allocator

//initialize tracking_allocator when ODIN_DEBUG
init :: proc() {
	when ODIN_DEBUG {
		mem.tracking_allocator_init(&tracking_allocator, context.allocator)
		context.allocator = mem.tracking_allocator(&tracking_allocator)
	}
}

// memory is allocated - if delete is left commented out, a leak should be printed by shutdown()
step :: proc() {
	when ODIN_DEBUG {
		context.allocator = mem.tracking_allocator(&tracking_allocator)
	}
	my_arr := make([dynamic]byte, 42)
	//defer delete(my_arr)
}

// print results of tracking_allocator when ODIN_DEBUG and then destroys it
shutdown :: proc() {
	when ODIN_DEBUG {
		context.allocator = mem.tracking_allocator(&tracking_allocator)
		defer {
			if len(tracking_allocator.allocation_map) > 0 {
				fmt.eprintf("=== %v allocations not freed: ===\n", len(tracking_allocator.allocation_map))
				for _, entry in tracking_allocator.allocation_map {
					fmt.eprintf("- %v bytes @ %v\n", entry.size, entry.location)
				}
			}
			mem.tracking_allocator_destroy(&tracking_allocator)
		}
	}
}

[xuul]

This conversation inspired me to add a new feature to my tracker to allow use of a global tracker. In the process I also finally made an internal improvement I’ve been wanting to make for a while now. This tracker also uses my afmt package, so if you are ok with that, give it a try. See main git page for all the details.

tracker

[hintron]

Nice! I had to figure out how to set a “shared” collection, since tracker imports shared:afmt, but after that, it just worked! (I’m installing third party libraries with Odyn, which has been great so far).

Here is the before (my boring tracking allocator) and after (super colorful rad tracking allocator):

Commit where I added it: Use a fancy tracking allocator! · hintron/traversal@db37d30 · GitHub

Unfortunately, it seems that karl2d doesn’t hook up the shutdown() function properly for wasm builds, so I haven’t been able to test either tracking allocator on the web.

[hintron]

I was able to get karl2d to run the shutdown() procedure, and now I have your allocation tracker running in WASM!:

See Call shutdown() in WASM builds by hintron · Pull Request #167 · karl-zylinski/karl2d · GitHub

[xuul]

That is excellent. I’m glad it is working for you. I’m guessing that second screenshot is a web view with the noansi flag set to true? Also, the tracker attempts to shorten paths to files without allocations using knowable indicators. I’m guessing in that second screenshot the project package is not named “traversal”?

@hintron

I had to figure out how to set a “shared” collection, since tracker imports shared:afmt

Both tracker and afmt do not need to be placed in shared. I documented that cause it seemed the path with the least friction. This means they are not technically in your project files. If you want them in your project so they could be shared with the source, just copy both folders to your “src” folder. Then in tracker.odin change the import for afmt to

import "../afmt"

then in your main.odin, change the tracker import to

import "tracker"

[xuul]

@xuul

I’m guessing in that second screenshot the project package is not named “traversal”?

EDIT: While working on this, I found an issue I was not aware of. Apparently #caller_location uses a ‘/’ as a separator for all paths regardless of os. I had not noticed until just now when working on this.

EDIT:
I think I’ve come up with a pretty good approach for trimming paths for all environments to prevent truncation (and better readability) in the tabled output. When anyone has time, please try it out and give feedback please.

tracker