Feedback on my first zig code and memory management

[pkazmier]

This weekend I wrote my first zig code. And while it only amounted to less than 100 lines of code, I learned a lot from this trivial exercise—coming from the world of garbage collected languages. I was hoping for some validation of my thought process to ensure I’m correctly thinking about memory management. Please note this is just a toy for myself to learn from and it is not complete.

I wrote two versions of a simple utility function to make an HTTP request that parses the resulting JSON into the supplied type. This is something I do often so I wanted to simplify the process a bit. As an example, if I wanted to parse some of the output from httpbin.org, I would do the following:

const url = "http://httpbin.org/anything";

const HttpBinResponse = struct {
    method: []const u8,
    origin: []const u8,
    url: []const u8,
    headers: std.json.Value,
};

pub fn main() !void {
    var gpa = std.heap.DebugAllocator(.{}).init;
    defer _ = gpa.detectLeaks();
    const allocator = gpa.allocator();

    var client = std.http.Client{ .allocator = allocator };
    defer client.deinit();

    const result = try jsonfetch.fetch2(&client, url, *HttpBinResponse);
    defer result.deinit();

    // Now use result.value.? which is a HttpBinResponse value.
}

The code can be found here. In root.zig, you’ll see two versions of JsonResult and fetch methods. The questions I have and would appreciate feedback on:

1. Is my thought process about conserving the extra allocation when parsing the JSON valid (see comment blocks in root.zig)? With a small change, I was able to avoid that extra allocation at the cost of some extra “private” fields in my result struct.
2. Is there an allocator that will tell me how many bytes have been requested over the lifetime of execution? I wanted to validate that version 2 does save a couple of allocations. I thought total_requested_bytes in DebugAllocator was what I was looking for, but it always printed zero. When looking at the code, I think this is the total outstanding bytes that have not been freed. So then I asked a couple of LLMs to write a tracking allocator and they both failed. Anyone have a simple way that I can use to validate number of bytes being allocated?
3. How can one find out the full list of possible errors that can be returned from a function? For example, in my fetch functions, I just use !JsonResult, but I’m guessing it’s better form to explicitly list what could fail? Or is it better to catch most of them and wrap them somehow into errors tailored for my function. How does one wrap errors?

My overall thoughts about zig after this weekend is that it is a lot of fun to write. In this simple project I was able to use comptime, optional types, error handling, pointers, etc … And reading the standard library is not daunting like it can be in some languages (i.e. rust, just reading rust standard library docs is daunting frankly). I’m really enjoying my zig adventure so far!

[milogreg]

Thanks for sharing! Here’s my answers for your questions:

1. You are right, less allocations will be made when producing JsonResult2. However, JsonResult2 will ‘own’ more total memory than JsonResult1, since it has to keep the entirety of buf alive even though it only needs the strings within buf.
2. To enable the tracking of total_requested_bytes, set enable_memory_limit to true in the DebugAllocatorConfig: std.heap.DebugAllocator(.{ .enable_memory_limit = true }).
3. Here’s a function you can use to log the inferred error set so you can declare them explicitly:

pub fn fnErrorSet(comptime function: anytype) []const std.builtin.Type.Error {
    const Function = @TypeOf(function);
    const function_info = @typeInfo(Function).@"fn";

    const Return = function_info.return_type.?;
    const return_info = @typeInfo(Return).error_union;

    const Err = return_info.error_set;
    const err_info = @typeInfo(Err).error_set.?;

    return err_info;
}

comptime {
    @compileLog(fnErrorSet(theFunctionIWantTheErrorSetOf));
}

As for ‘wrapping’ errors, I am assuming you are talking about Go-style error handling where each function the error is propagated through tacks on a bit of info (error: failed to parse: invalid string: invalid char). This pattern isn’t as well-supported in Zig since errors don’t have payloads, so I’d recommend either propagating errors up or returning a new error if the propagated error wouldn’t make sense to the caller.

[pkazmier]

@milogreg

To enable the tracking of total_requested_bytes, set enable_memory_limit to true in the DebugAllocatorConfig: std.heap.DebugAllocator(.{ .enable_memory_limit = true }).

Thank you for all of the feedback. Re: this item, I had tried that as well, but the value was always zero at the end. I think maybe that total value is how many have been requested minus the ones that have been freed. So in a well behaved program, is it supposed to be zero at the end?

[KilianHanich]

main

@pkazmier

defer _ = gpa.detectLeaks();

Fyi: detectLeaks() doesn’t deinit the DebugAllocator, but just detect if there are any leaks.
This is something I consider slightly unusual and because of that a place where imo a comment, which explains the decision, is appropriate.

But generally you could just use an ArenaAllocator for the entire application. But if you want to learn how to deal with manual memory management generally, not using them is kinda understandable in this case, even if I wouldn’t do it in “production” code. If you really want to go all the way (as in really want to fully do it yourself and keep track of all code paths) you may even want to go and not use defer either.

Also, you should go and write tests.

About your first attempt

http_result.status != std.http.Status.ok → http_result.status != .ok

For what you wanted to do here, I think it’s mostly fine, even if not all that fast, which I think you tried to tackle in your second attempt. But if your main goal is not speed but small memory usage, this is can be better than your second attempt depending on the specific Ts in question.

About your second attempt

1. Your result struct is unnecessarily large. It saves 2 instances of the result value and a buffer.
2. If parseFromSlice fails, you leak your buffer.

The first point can be improved by building the Result like this:

fn JsonResult3(comptime T: type) type {
    return struct {
        status: std.http.Status,
        value: ?T,
        // using underscore in the same way you do
        _arena: ?std.heap.ArenaAllocator,

        pub fn deinit(self: @This()) void {
            if (self._arena) |arena| {
                arena.deinit();
            }
        }
    };
}

I let you figure out how to do the fetch function as an exercise.

To fix the second point, you should look into errdefer.

Figuring out the error set

I personally just put in one error (often OutOfMemory) and the let the compiler tell me which I missed in most cases. Sometimes I can also just tell from the code.

[pkazmier]

@KilianHanich

Fyi: detectLeaks() doesn’t deinit the DebugAllocator, but just detect if there are any leaks.
This is something I consider slightly unusual and because of that a place where imo a comment, which explains the decision, is appropriate.

Ah, good to know. I’ll add another defer with the deinit.

@KilianHanich

But generally you could just use an ArenaAllocator for the entire application. But if you want to learn how to deal with manual memory management generally, not using them is kinda understandable in this case, even if I wouldn’t do it in “production” code. If you really want to go all the way (as in really want to fully do it yourself and keep track of all code paths) you may even want to go and not use defer either.

I hadn’t really thought about using the ArenaAllocator, but I was more focused on learning about memory management as this is very new to me coming from languages like Python, Go, Lua, Erlang, etc … I’ll try a version 3 to rewrite per your later suggestion.

@KilianHanich

Also, you should go and write tests.

Of course. I am still very much iterating and experimenting and trying to grasp core concepts. My plan is to add options and play around with some other things as well.

@KilianHanich

Your result struct is unnecessarily large. It saves 2 instances of the result value and a buffer.

Can you help elaborate on this point? I’m not sure I understand. Before I decided to add both _buf and _parsed in there, I asked myself a couple of questions.

1. Should I use pointers here? I decided no because ArrayList (after looking in the standard library code) is a simple struct with a slice, so I thought copying here is trivial and not really moving memory around. As for Parsed, it, too, was a simple struct that contains the value of T, but in my case, my T is *HttpBinResult, so I thought this is just a simple copy of a pointer.
2. Am I going to have two copies, more or less, of the same data in memory hanging around? I thought the answer was no. I thought by using .alloc_if_needed with the JSON parser meant it would only save slices for all of the strings that point to the original _buf. I suppose this line of reasoning (if it’s even correct) would be largely dependent on the ratio of strings in the JSON response. For JSON with minimal to no strings, then I would be saving two copies as you mention. But for JSON responses comprised primarily of strings, this wouldn’t be too bad, would it?

I’m still trying to wrap my head around how using the arena avoids having two copies of the data in memory. Perhaps I don’t understand what you were intending to convey. The reason I’m confused is that with the arena, I’m not really sure how that avoids have both the _buf and Parsed in memory at the same time. I envision this arena version of fetch to immediately create an arena with the allocator supplied by the caller, then I use that arena everywhere I use the allocator in the function now (the array list and the parse from slice), save a reference to it, and then deinit as you show in your version 3 version. If the deinit doesn’t happen until the end when the user calls it, then isn’t it more or less the same as my version 2?

And maybe the prior paragraph is moot because I think what you are pointing out is that it is not really accomplishing what I had intended, saving memory (unless it’s a heavily string-based JSON response) at the cost of complicating the implementation because I was so hyper-focused on minimizing allocations. I think this new world of memory management has me thinking like allocations are the bane of all existence. I’m obviously over-indexing on this. So, in your opinion, you said version 1 was okay, but you mentioned slow. Is this third version the best of both worlds and I just don’t understand it yet?

Thank you so much for helping me think through these memory management issues. It’s exactly what I was hoping for as I don’t want to walk away with an incorrect mental model of how this stuff works.

[milogreg]

You’re right! The docs for DebugAllocatorConfig could be a little clearer…

There’s another way, though:

pub fn main() !void {
    var gpa: std.heap.DebugAllocator(.{}) = .init;
    defer _ = gpa.deinit();
    const gpa_allocator = gpa.allocator();

    var failer: std.testing.FailingAllocator = .init(gpa_allocator, .{});
    const allocator = failer.allocator();

    const my_alloc = try allocator.alloc(u8, 1000);
    std.debug.print("{}\n", .{failer.allocated_bytes});
    allocator.free(my_alloc);
    std.debug.print("{}\n", .{failer.allocated_bytes});
}

std.testing.FailingAllocator also gives some other useful diagnostics like number of allocations, frees, etc.

[pkazmier]

Thank you. I will give this a try.

[KilianHanich]

@pkazmier

@KilianHanich

Fyi: detectLeaks() doesn’t deinit the DebugAllocator, but just detect if there are any leaks.
This is something I consider slightly unusual and because of that a place where imo a comment, which explains the decision, is appropriate.

Ah, good to know. I’ll add another defer with the deinit.

DebugAllocator.deinit does the check for you. You only need one defer here.

@pkazmier

Can you help elaborate on this point?

Ok, in your struct you have the following members:

• status: std.http.Status
• value: ?T
• _buf: std.ArrayList(u8)
• _parsed: ?std.json.Parsed(T)

My proposal has:

• status: std.http.Status
• value: ?T
• arena: ?std.heap.ArenaAllocator

I am going to ignore the members which are identical here.

A std.ArrayList always saves:

• a slice
• a usize
• an std.mem.Allocator

A std.json.Parsed always saves:

• the value T
• a pointer to a std.heap.ArenaAllocator

An arena allocator always cleans up all of the memory, which was allocated through it, when it gets cleaned up. So if the allocator from the ArrayList refers to an ArenaAllocator, you don’t need to deinit the ArrayList itself if you deinit the arena. (This is the reason why my deinit doesn’t do that.)

As far as the memory concerned, my proposal requires the size of an std.http.Status, the size of however the wanted value is once + the overhead from it being an optional and the size of an arena.
Your version requires the size of an std.http.Status, the size of however the wanted value is twice + once the overhead from one being an optional, the size of a pointer to an arena, the size of a slice, the size of a usize and the size of an std.mem.Allocator.

A std.json.Parsed is essentially just a convenience wrapper around the value and something to keep track of its required memory during the parsing. But you can do a better job yourself here. Look up how std.json.parseFromSlice is implemented (it’s actually quite short) and dig down until you encounter a “leaky” function.

[tholmes]

@pkazmier

3. How can one find out the full list of possible errors that can be returned from a function? For example, in my fetch functions, I just use !JsonResult, but I’m guessing it’s better form to explicitly list what could fail? Or is it better to catch most of them and wrap them somehow into errors tailored for my function. How does one wrap errors?

This is done by merging error sets.
It’s best explained using the official Zig documentation.
In your case, to create a complete error set for your fetch function, you might do the following:

pub const FetchStdError = std.http.Client.ConnectError || 
	std.http.Client.ConnectTcpError || 
	std.http.Client.ConnectUnixError || 
	std.http.Client.RequestError || 
	std.json.ParseError(std.json.Scanner) || 
	std.mem.Allocator.Error;

I would also recommend defining a custom error for your fetch function:

pub const FetchCustomError = error.HttpStatusError;

You could then merge these two sets together to create a final “FetchError” set that describes all of the errors you could possibly return.

The reason why I suggest creating this new error is because it would make for a convenient helper case in your fetch functions:

if (http_result.status != .ok) {
    std.log.err("HTTP request failed with code {d} {t}", .{@as(u32, @intFromEnum(http_result.status)), http_result.status});
    buf.deinit();
    return FetchError.HttpStatusError;
}

This would mean you’d no longer need to store the HTTP result code in the result struct, and furthermore you could make the JSON value no longer optional since it can now never be null (the function would error out instead).

@pkazmier

1. Is my thought process about conserving the extra allocation when parsing the JSON valid (see comment blocks in root.zig)? With a small change, I was able to avoid that extra allocation at the cost of some extra “private” fields in my result struct.

It doesn’t really make sense to solve temporary double-allocation by having permanent double-allocation.
It’s actually vastly better to let “alloc_always” do its thing, because it means you don’t need to have your JSON values be backed by the original buffer - meaning most of the syntax can just be dropped and the raw parsed values can stay allocated.
In other words, it’s less like you’re duplicating the entire JSON buffer during the parsing process, and more like “if we encounter a string literal, allocate it”.
That’s why it’s actually better to let the buffer be freed after the function returns.

It’s worth noting that there’s also more than one way of fixing this. An elegant way would be to make the std.http.Client negotiate a connection instead of sending a single GET request, so you can utilise the connection’s streaming output with std.json.Scanner and simply immediately copy-and-forget each token as you parse it.
This would be a more ideal, robust solution for large JSON files.

But since you’re not working with large JSON files at the moment, and you don’t necessarily need to return the HTTP status code, you can actually just have your fetch function return std.json.Parsed(T) now that we’ve got rid of all other struct members.

[pkazmier]

Thank you for your explanation and thank you very much for taking the time to help me!

[pkazmier]

Thank you very much for the feedback. I’ve decided to take the approach suggested to simplify my utility function. I’ve created my own custom errors to hide the lower level errors, which just seems like better design anyways. I’m still trying to wrap my head around the fact that errors cannot have any context associated with them like a Python exception or Go error. I’m just not used to having library functions print to the screen. So I added a scope and lowered the level to debug for those. If my utility function does return an error, then the main program prints to stderr and exits a status code of 1. Here is the new code.

[pkazmier]

I added tests and a README for this trivial wrapper to std.http.Client.fetch and std.json.parseFromSlice. Thank you again for helping answer my newbie questions as this was literally the first lines of zig I’d ever written.

GitHub - pkazmier/jsonfetch: Zig wrapper to make HTTP requests and parse JSON...