search

ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
33.86k stars 2.48k forks source link

wasi thread spawning should use memory growing intrinsics rather than accepting an allocator #19383

Open andrewrk opened 6 months ago

andrewrk commented 6 months ago

Follow-up I noticed when reviewing #19348.

https://github.com/ziglang/zig/blob/54c08579e4859673391843182aa2fd44aabbf6cf/lib/std/Thread.zig#L835-L837

Used here:

https://github.com/ziglang/zig/blob/54c08579e4859673391843182aa2fd44aabbf6cf/lib/std/Thread.zig#L910

However, if you look above, it's one allocation that is already page-aligned... there's no point in going through an allocator here. Using @wasmMemoryGrow is simpler and more efficient.

I don't think std.Thread API should accept an allocator as an option.

cc @jedisct1 @Luukdegram

Luukdegram commented 6 months ago

The main reason for the allocator is to prevent leaking as much memory. There's currently no memory.shrink instruction (yet). This means that each thread spawn would leak its memory upon join. By storing the allocator and using that, at least the allocator can be notified of the freeing of said memory region and is allowed to re-use that memory.

If we are fine with leaking at least a page (possibly more depending on stack size) on each thread spawn, it would at the very least simplify the join and detach implementations by a lot.