Allocator or buffer?

Explain
1

Hi! I’m currently writing:

pub fn init(allocator: Allocator, path: []const u8) !Database {
        const path_sentinel = try allocator.dupeSentinel(u8, path, 0);
        defer allocator.free(path_sentinel);

        var db = try sqlite.Db.init(.{
            .mode = .{ .File = path_sentinel },
...

And I’m considering either allocator or buffer is better for my function?

On a deeper level, I come from high-level language. Thus, using allocatoris is regarded as a heavy and unnatural thing for me. I look up in std, the two implementation coexist, while even some api provide both two choices.

So, could anybody teach me when I need to use an allocator and what is the difference between buffer and allocator?

2

We can’t tell you that, that’s a core part of figuring out how your code wants to work, so it’s your job to answer.

That being said, given the very few lines you’ve shown us, maybe your function should accept path: [:0]const u8 instead to avoid the duplication.

4 Likes
3 
const std = @import("std");
const sqlite = @import("sqlite");

const Allocator = std.mem.Allocator;

pub const Database = struct {
    db: sqlite.Db,

    pub fn init(allocator: Allocator, path: []const u8) !Database {
        const path_sentinel = try allocator.dupeSentinel(u8, path, 0);
        defer allocator.free(path_sentinel);

        var db = try sqlite.Db.init(.{
            .mode = .{ .File = path_sentinel },
            .open_flags = .{
                .write = true,
                .create = true,
            },
            .threading_mode = .Serialized,
        });
        errdefer db.deinit();

        try applyPragmas(&db);

        return .{
            .db = db,
        };
    }

    pub fn deinit(self: *Database) void {
        self.db.deinit();
    }
};

fn applyPragmas(db: *sqlite.Db) !void {
    try db.pragma([128:0]u8, .{}, "journal_mode", "wal");
    try db.pragma(void, .{}, "busy_timeout", "3000");
    try db.pragma(void, .{}, "synchronous", "NORMAL");
    try db.pragma(void, .{}, "foreign_keys", "1");
}

Well, thanks for your attention. I’m using zig-sqlite pkg and its real api need a c style string. However, the path parameter may mainly comes from literal or environment variables, so I think []u8 is better for path.

4

The caller is responsible for getting the path, why cant the caller be responsible for ensuring it meets your api’s requirements?

Generally you should prefer allocators, they are more flexible, especially if the buffer size needed is not known by the caller (that is not the case for your situation).

The caller could pass a FixedBufferAllocator which allocates from a buffer, though it may still be slower than directly using a buffer since the interface has overhead 1) from runtime known function calls 2) it provides a higher level api that has to convert to the lower level api of the vtable functions.
It is possible the overhead could be optimised away.

2 Likes
5

both literals and environment variables are usually 0-terminated, last i checked…

6

Environment variables being nul terminated is platform dependant, so the agnostic api provides non terminated slices.

1 Like
7

Thank you :grinning_face:, it seems requiring caller to meet requirements is better in my situtaion. What’s more, this reminds me allocator is much more flexible than buffer especially when the parameter needs IO.

8
  1. It’s a leaky abstraction. Zig provides slices as a natural way to model the argument.
  2. It forces consumers to solve a puzzle how to obtain nul-terminated path.
  3. It doesn’t matter from performance perspective. The whole domain could be opinionated and provide nice slice API instead of thin C wrapper which solves nothing.

I get that stdlib could leak details, (oh god ZON parser requiring [:0]u8) because it doesn’t know in which context it could be used. But IMHO “Software You Can Love” should go alongside with “Libraries you Can Love”.

1 Like
9

Sadly, there is no convention to communicate that allocator is required only for fn call lifetime. So it’s always a guess work is it safe to pass a caller stack arena (for example). Especially for your case wrapper should ensure sqlite doesn’t keep references.

10

you can also just go with both:

//alloc is only used to convert path to a 0-terminated slice, which is freed when returning
pub fn initAlloc(alloc: Allocator, path: []const u8) !Database {
  const path_sentinel = try alloc.dupeSentinel(u8, path, 0);
  defer alloc.free(path_sentinel);
  return initZ(alloc, path);
}

pub fn initZ(path: [:0]const u8) !Database {
  var db = try sqlite.Db.init(.{
    .mode = .{ .File = path },
...
}

and then later on, if there’s a future reimplementation of the sqlite api without a null terminator requirement, you could do:

pub fn init(path: []const u8) !Database {
  var db = try sqlite_new.Db.init(.{
    .mode = .{ .File = .{ .ptr = path.ptr, .len = path.len } }, //hypothetical api
...
}

//deprecated, use init([]const u8)
pub fn initAlloc(_: Allocator, path: []const u8) !Database {
  return init(path);
}

//deprecated, use init([]const u8)
pub fn initZ(path: [:0]const u8) !Database {
  return init(path);
}

and this way the api still works for existing code but the allocation is gone, and people who didn’t use an allocator before with [:0]const u8 still don’t need to

4 Likes
11

and one thing slices support is enforcing termination as part of the type :3. If that detail is always required why is it bad to leak that requirement?

An easy puzzle.

but it could from a memory perspective if in a constrained environment, the caller will be able to terminate the path at least as efficiently as doing it internally if not more efficient.

We don’t know where Op is running this code. Though it is reasonable to assume they would have mentioned if it was anywhere but a standard modern pc.

2 Likes
12

The general question probably needs a couple of blog posts to answer, but, in this specific case, I think the right answer is “do what std does”:

https://codeberg.org/ziglang/zig/src/commit/1bcd8d9fe60f72849254b7f74d9ea0f48eae6aaa/lib/std/Io/Threaded.zig#L4819-L4820

    var path_buffer: [posix.PATH_MAX]u8 = undefined;
    const sub_path_posix = try pathToPosix(sub_path, &path_buffer);
4 Likes
13

The path may need to live longer than init, in which case it should be done by the caller.

or embed the array into the type and do an inplace init :face_with_open_eyes_and_hand_over_mouth: but you probably dont want to increase your types size that much

1 Like
14

Caller also could assume that stack allocated buffer is ok. Boom. It doesn’t solve the issue.

15

a stack allocated buffer in the callers frame may indeed be okay, though that is unlikely and the caller would know it, and either use a global buffer or heap allocate.

The point is a stack buffer in init is most probably not okay.

16

I tend to agree that for this particular case. [:0]u8 is a better option. It’s too much hassle to provide an allocator for a mere dup when caller could make own easy abstraction.

1 Like
17

at that point the best option is to mention how long the memory needs to be kept alive (e.g. ///path must not be freed until the returned database is deinited or ///alloc must be an arena that may only be deinited after the db is deinited, etc.) and expect api users to read documentation comments, which is reasonable imo

EDIT: note that in this case, the example i posted above with the initAlloc redirecting to init would still need to do a dupe with the allocator to preserve the lifetime constraints

1 Like