Well known Zig allocator idiom -
is a design pattern where functions requiring memory take an
std.mem.Allocatorinstance as an argument, delegating memory allocation decisions to the caller.
I am asking about opposite direction - give allocator
In my current project I’d like that all clients of my code use the same allocator.
As possible solution - client gets allocator from my library and uses it
Are there any pitfalls in this decision?
Define
I assume the reason you want them to use the same allocator is because you do some memory management in your library.
Just document what it does with allocations, allocations are inherently tied to their allocator, this is not unique to your library.
It is a rule everyone should be following, and it’s easy to follow, as it’s uncommon to have more than 2 allocators available to any piece of code.
So, there are no pitfalls, and it is the norm. Unless I am misunderstanding.
To be clear, I am talking about using the same allocator. On the other hand, forcing the use of an allocator provided by your library is limiting, especially for enforcing a practice that is already used.
just for clarification - my library gets allocator regular way - client should provide it
Because library initiates threads and long-lived “objects”
I asked client to provide gpa compatible allocator.
This one library will return to the client code
I use the “runtime_safety only fields” pattern extensively in my personal code.
In this pattern, in debug mode or releaseSafe mode, the first client’s allocator is lazily loaded into the global context, and subsequent allocators passed by other clients are checked against the allocator recorded in the global context using assertions. In releaseFast mode, these checks do not exist.
pub const runtime_safety = switch (@import("builtin").mode) {
.Debug, .ReleaseSafe => true,
.ReleaseFast, .ReleaseSmall => false,
};
pub const SafetyCheck = struct {
alloc: ?std.mem.Allocator,
};
pub const MyLib = struct {
...,
safety_check: if (runtime_safety) SafetyCheck else void = if (runtime_safety) .{ .alloc = null } else {},
};
Am I missing something or… you are just talking about making custom allocator? Like, you can always make a var lib_gpa_state : mylib.AwesomeAllocator = .init(); and then people can use lib_gpa_state.allocator() whenever std.mem.Allocator is required.
Like… how allocators in std.heap are implemented
pub const Library = struct {
gpa: Allocator = undefined,
pub fn Create(gpa: Allocator) !*Library {
const ret = try gpa.alloc(Library);
ret.gpa = gpa;
return ret;
}
pub fn giveAllocator(lib: *Library) Allocator {
return lib.gpa;
}
pub fn asyncSend(lib: *Library, buf: []const u8) void {
......................................
}
};
pub const Client = struct {
lib: *Library = undefined,
pub fn Create(lib: *Library) !*Client {
const ret = try lib.giveAllocator();
ret.lib = lib;
return ret;
}
pub fn process(cln: *Client) void {
.....................................
const blob: []const u8 = try cln.lib.giveAllocator().alloc(u8, 1024);
cln.lib.asyncSend(blob);
.....................................
}
};
..............................................
var lib: *Library = try Library.Create(std.testing.allocator);
........................
.........................
var client: *Client = try Client.Create(lib);
.........................
.........................
client.process();
.........................
We are talking about using the same allocator
After finish of send on another thread, lib will use own allocator for destroy
I see, so buf passed to asyncSend must be allocated by the same allocator passed in .Create in your case… I would just let lib.gpa be a field to be accessed and encode that as a requirement
pub const Library = struct {
gpa: Allocator = undefined,
pub fn init(gpa: Allocator) Library {
return .{ .gpa = gpa };
}
// buf must be allocated by `lib.gpa`
pub fn asyncSend(lib: *Library, buf: []const u8) void {
// ...
}
};
If this sounds like mistake-prone, I could also have easier-to-use, less-footgun APIs in common usage. Just drop down to more granularities when you know you need it.
// buf must be allocated by `lib.gpa`
pub fn asyncSendAssumeAllocated(lib: *Library, buf: []const u8) void {
// ...
}
// duplicate buffer with `lib.gpa`
pub fn asyncSend(lib: *Library, buf: []const u8) !void {
return lib.asyncSendAssumeAllocated(try lib.gpa.dupe(u8, buf));
}
etc etc. On client-side:
var gpa: std.mem.Allocator = std.testing.allocator;
var lib: Library = .init(gpa);
var blob: std.ArrayList(u8) = try .initCapacity(lib.gpa, 1024);
lib.asyncSend(blob.items);
EDIT: I would have make the more footgun API more verbose. edited accordingly
EDIT2: and ideally, you would want to have some std.debug.assert to check the requirements as well. Though I am not sure how to check if that block is allocated by the same allocator
We had long allocator related discussion
see Attributes
For my project
mallocReplacement == true
will be good enough
Is using wrong allocator that much of a footgun that requires so much gymnasiums to solve and enforce in code? Surely your software does more things other than allocating memory… right?
For the record, I have never had problems with allocator. If there are performance issues, the answer almost always is not better allocators, but better allocations. Maybe your concurrent or lock-free APIs/algorithms should just take in pre-allocated memories? Maybe batch allocate all the things at a start of a parallel loop? Maybe not call allocator.alloc every loops / on every threads? Maybe you need a memory pool?
etc etc
(Same as some C++/Rust code I have seen in the wild. Maybe your programs have better things to do other than initializing objects, or wrapping things in newtypes and traits…?)
It is always a foot gun, but you’re right that it is unnecessary to enforce it as it’s not that big of an issue, and will be caught if using DebugAllocator’s, which you should be in debug modes (unless they are way too slow, then it’s understandable to use something else).
Either way, when memory bugs happen the first thing you check is if you used the wrong allocator, if that’s even possible in your code.
Regardless, the main reason they are storing the allocator is to free the allocated memory from another thread, which is a valid solution.
I think you need to identify what is being passed and whether or not it needs to be passed in. In this case, it seems like when you initialize the library you could add a field that is a buffer, or have the client provide the buffer if you want them to control the size.
The instead of passing the buffer in, you could have them use the internal library buffer and pass a slice of that buffer in.
This is from observing the code you provided, but ultimately I don’t think your problem is really that you need the allocator to be passed from the library to the client, but rather that there is certain data that should be owned by the library that you are instead trying to pass around. You may want to rethink your ownership and data lifetimes here.
Since the library only accepts buffers being allocated in certain ways, it should allocate the buffers itself and hands them back to the clients to use. These buffers are library owned. See sample below,
pub const Library = struct {
gpa: Allocator,
pub fn getBuf(lib: *const Library, size: usize) ![]const u8 {
return try lib.gpa.alloc(u8, size);
}
/// Client must pass in buf allocated with getBuf()
pub fn asyncSend(lib: *Library, buf: []const u8) void {
…
// much later after the async operation completed
lib.gpa.free(buf);
}
};
pub const Client = struct {
lib: *Library,
pub fn process(cln: *Client) void {
const blob: []const u8 = try cln.lib.getBuf(1024);
… fill blob …
cln.lib.asyncSend(blob);
}
};
My fault
Library actually is long lived multithreaded code (service)
Main functionality - asynchronous message passing
via TCP sockets and Unix Domain Sockets.
I did not force client to use the same allocator, but help
to use the same allocator if she/he wants
Interface of this service will be done in allocator if style =>
client code saves it within own structs.
So client always has access to allocator used by all participants…
Rough example of main interface Ampe - async message passing engine:
pub const Ampe = struct {
.........................................
/// Gets a message from the internal pool based on the allocation strategy.
pub fn get(ampe: Ampe, strategy: AllocationStrategy) status.AmpeError!?*message.Message {
.........................................
}
/// Returns a message to the internal pool.
pub fn put(ampe: Ampe, msg: *?*message.Message) void {
.........................................
}
/// Creates new Channels.
pub fn create(ampe: Ampe) status.AmpeError!Channels {
.........................................
}
/// Destroys Channels, stops communication, and frees memory.
pub fn destroy(ampe: Ampe, chnls: Channels) status.AmpeError!void {
.........................................
}
/// Gets allocator used by engine for all memory management.
pub fn getAllocator(ampe: Ampe) Allocator {
.........................................
}
Ampe acts as allocator, but instead of dealing with memory, it’s allocates/de-allocates main engine resources:
Rough example of the caller code:
const TofuServer = struct {
const Self = @This();
ampe: Ampe = undefined,
chnls: ?tofu.Channels = undefined,
cfg: Configurator = undefined,
helloBh: message.BinaryHeader = undefined,
connected: bool = undefined,
pub fn create(engine: Ampe, cfg: *Configurator) status.AmpeError!*Self {
const allocator = engine.getAllocator();
const result: *Self = allocator.create(Self) catch {
return status.AmpeError.AllocationFailed;
};
errdefer allocator.destroy(result);
result.* = try Self.init(engine, cfg);
try result.createListener();
return result;
}
fn createListener(server: *Self) status.AmpeError!void {
var welcomeRequest: ?*Message = server.*.ampe.get(tofu.AllocationStrategy.always) catch unreachable;
defer server.*.ampe.put(&welcomeRequest);
server.*.cfg.prepareRequest(welcomeRequest.?) catch unreachable;
var initialBh = server.*.chnls.?.sendToPeer(&welcomeRequest) catch unreachable;
var welcomeResponse: ?*Message = server.*.chnls.?.waitReceive(tofu.waitReceive_INFINITE_TIMEOUT) catch {
return err;
};
defer server.ampe.put(&welcomeResponse);
welcomeResponse.?.bhdr.dumpProto("server recv ");
if (welcomeResponse.?.bhdr.status == 0) {
return;
}
status.raw_to_error(welcomeResponse.?.bhdr.status) catch {
return err;
};
}
}
You can see that
Example of ampe creation:
var eng: *Engine = try Engine.Create(gpa, options);
defer eng.Destroy();
const ampe: Ampe = try eng.ampe();
So engine does not return own alocator, it returns allocator provided by caller during creation
I have a lot of questions about your API.
Firstly, why does the caller need to call engine.getAllocator()?
Why can’t engine have a pub const allocator: std.mem.Allocator = std.heap.smp_allocator; declaration (which can be variable or constant as you desire) which the caller (and the Ampe definition) can directly access as engine.allocator?
Secondly, why is Ampe.get() able to return null when it can already error out?
Why do message pointers need to be optional? Even if you’re interacting with a (translated) C library, you can test if a pointer is null by saying const ptr = cptr orelse return error.CNullPtr;
On a related note, why does Ampe.put() need a pointer to an optional pointer to a message? Why is one layer of pointers not good enough?
Thirdly, in the Zig Zen it’s said that “Resource allocation may fail; resource deallocation must succeed.”
As I understand it, Ampe.create() is resource allocation, and Ampe.destroy() is resource deallocation.
So why can Ampe.destroy() fail?
Fourthly, why are you erasing error information from functions you call in TofuServer.create_listener()?
I understand if you want all the returned errors to be part of the same error set - but, you can do something clever and concatenate multiple error sets - this would allow you to simply use the try keyword instead of catch return err;.
I’d also like to point out that when you’re passing a pointer to a struct into a function (as in createListener(server: *Self) status.AmpeError!void), you don’t actually need to dereference the pointer to get/set members of that struct.
Haha, this is the one thing I like about OP’s style. I think this way of writing is more explicit, allowing you to see at a glance whether I am operating on a pointer. Besides, I have always suspected that this syntactic sugar might occasionally reveal some footguns and could potentially be removed.
Lot of thanks for the attention to detail.
I really miss the reviewer.
I’ll try to explain the reasons.
Let’s start from simplest:
Sometimes I did it , sometimes don’t. I’ll probably make this a rule.
The same way I am thinking less to use type inference.
With triple CLion/ZigBrain plugin/ ZLS I have all information, but when I try to uinderstand my own code in the browser, sometimes I feel not comfortable.
Guilty. It’s example, but mindset still of ampe developer.
I’ll fix it
“Sh*t happens”. We are talking about long lived multithreaded code with non closed sockets etc. I’d like to inform caller…
If resource deallocation always succeed, we can use it with defer/errdefer. I undersand reason for Zen.
If caller won’t to get error during destroy, he/she will wrap it on own code with catch and possible additional log about failure
Ampe is “interface”:
pub const Ampe = struct {
ptr: ?*anyopaque,
vtable: *const vtables.AmpeVTable,
...............................................................
};
Is it OK to add allocator as a field ?
pub const Ampe = struct {
ptr: ?*anyopaque,
vtable: *const vtables.AmpeVTable,
allocator: std.mem.Allocator,
...............................................................
};
According to design, null is valid value for the message, all apis are dealing with this value.
null is returned when pool is empty, it is not error, it’s intent.
the same way Zig ArrayHashMap getPtr returns null for non existing value.
Error will be returned for example if allocation really failed, or system is in shutdown state.
Now let’s talk about *?*Message
var welcomeRequest: ?*Message = server.*.ampe.get(tofu.AllocationStrategy.always) catch unreachable;
defer server.*.ampe.put(&welcomeRequest);
...........................................................
...........................................................
_ = server.*.chnls.?.sendToPeer(&welcomeRequest) catch unreachable;
First of all we got welcomeRequest from the pool.
And immediatelly write defer in order to release message to the pool.
welcomeRequest may be null, but put “knows” about this possibility: If welcomeRequest != null, it returns welcomeRequest.? to the pool and sets welcomeRequest to null (remember *?).
sendToPeer also works in the same manner:
For any case and any errors within function defer will return message to the pool if it was not ‘captured’
So
Simplest ownership moving in async environment
It would be interesting to discuss this approach.