From 2bc17a33fbc9de0d4ec4fdb5d30ee38d1a470fc8 Mon Sep 17 00:00:00 2001 From: Stephen Gutekanst Date: Fri, 19 Apr 2024 17:11:43 -0700 Subject: [PATCH] module: correct alignment of dispatched arguments; move stack space to caller Signed-off-by: Stephen Gutekanst --- src/core/main.zig | 6 ++++-- src/module/main.zig | 3 ++- src/module/module.zig | 49 ++++++++++++++++++++++++++++++------------- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/core/main.zig b/src/core/main.zig index c94bbc45..efded542 100644 --- a/src/core/main.zig +++ b/src/core/main.zig @@ -10,13 +10,15 @@ const platform = @import("platform.zig"); const mach = @import("../main.zig"); pub var mods: mach.Modules = undefined; +var stack_space: [8 * 1024 * 1024]u8 = undefined; + pub fn initModule() !void { // Initialize the global set of Mach modules used in the program. try mods.init(std.heap.c_allocator); mods.mod.mach_core.send(.init, .{}); // Dispatch events until this .mach_core.init_done is sent - try mods.dispatch(.{ .until = .{ + try mods.dispatch(&stack_space, .{ .until = .{ .module_name = mods.moduleNameToID(.mach_core), .local_event = mods.localEventToID(.mach_core, .init_done), } }); @@ -29,7 +31,7 @@ pub fn tick() !bool { mods.mod.mach_core.send(.main_thread_tick, .{}); // Dispatch events until this .mach_core.main_thread_tick_done is sent - try mods.dispatch(.{ .until = .{ + try mods.dispatch(&stack_space, .{ .until = .{ .module_name = mods.moduleNameToID(.mach_core), .local_event = mods.localEventToID(.mach_core, .main_thread_tick_done), } }); diff --git a/src/module/main.zig b/src/module/main.zig index 92778500..7d963671 100644 --- a/src/module/main.zig +++ b/src/module/main.zig @@ -102,5 +102,6 @@ test "entities DB" { //------------------------------------------------------------------------- // Send events to modules world.mod.renderer.sendGlobal(.tick, .{}); - try world.dispatch(.{}); + var stack_space: [8 * 1024 * 1024]u8 = undefined; + try world.dispatch(&stack_space, .{}); } diff --git a/src/module/module.zig b/src/module/module.zig index 02e32c84..96c7ac5c 100644 --- a/src/module/module.zig +++ b/src/module/module.zig @@ -114,6 +114,7 @@ pub fn Modules(comptime modules: anytype) type { module_name: ?ModuleID, event_name: EventID, args_slice: []u8, + args_alignment: u32, }; const EventQueue = std.fifo.LinearFifo(Event, .Dynamic); @@ -142,9 +143,9 @@ pub fn Modules(comptime modules: anytype) type { break :blk std.ascii.eqlIgnoreCase(s, "true"); } else false; - // TODO: custom event queue allocation sizes m.* = .{ .entities = entities, + // TODO(module): better default allocations .args_queue = try std.ArrayListUnmanaged(u8).initCapacity(allocator, 8 * 1024 * 1024), .events = EventQueue.init(allocator), .mod = undefined, @@ -152,7 +153,7 @@ pub fn Modules(comptime modules: anytype) type { }; errdefer m.args_queue.deinit(allocator); errdefer m.events.deinit(); - try m.events.ensureTotalCapacity(1024); + try m.events.ensureTotalCapacity(1024); // TODO(module): better default allocations // Default initialize m.mod inline for (@typeInfo(@TypeOf(m.mod)).Struct.fields) |field| { @@ -274,6 +275,7 @@ pub fn Modules(comptime modules: anytype) type { .module_name = module_name, .event_name = event_name, .args_slice = m.args_queue.items[m.args_queue.items.len - args_bytes.len .. m.args_queue.items.len], + .args_alignment = @alignOf(@TypeOf(args)), }); } @@ -305,8 +307,12 @@ pub fn Modules(comptime modules: anytype) type { }; /// Dispatches pending events, invoking their event handlers. + /// + /// Stack space must be large enough to fit the uninjected arguments of any event handler + /// which may be invoked, e.g. 8MB. It may be heap-allocated. pub fn dispatch( m: *@This(), + stack_space: []u8, options: DispatchOptions, ) !void { const Injectable = comptime blk: { @@ -327,11 +333,12 @@ pub fn Modules(comptime modules: anytype) type { } @compileError("failed to initialize Injectable field (this is a bug): " ++ field.name ++ " " ++ @typeName(field.type)); } - return m.dispatchInternal(options, injectable); + return m.dispatchInternal(stack_space, options, injectable); } pub fn dispatchInternal( m: *@This(), + stack_space: []u8, options: DispatchOptions, injectable: anytype, ) !void { @@ -339,8 +346,6 @@ pub fn Modules(comptime modules: anytype) type { // TODO: parallel / multi-threaded dispatch // TODO: PGO - // TODO(important): we may exceed stack space here, consider moving to heap. - var buf: [8 * 1024 * 1024]u8 = undefined; while (true) { // Dequeue the next event m.events_mu.lock(); @@ -349,12 +354,16 @@ pub fn Modules(comptime modules: anytype) type { return; }; - // Pop the arguments off the stack, so we can release args_slice space. + // Pop the arguments off the ev.args_slice stack, so we can release args_slice space. // Otherwise when we release m.events_mu someone may add more events' arguments // to the buffer which would make it tricky to find a good point-in-time to release // argument buffer space. - @memcpy(buf[0..ev.args_slice.len], ev.args_slice); - ev.args_slice = buf[0..ev.args_slice.len]; + const aligned_addr = std.mem.alignForward(usize, @intFromPtr(stack_space.ptr), ev.args_alignment); + const align_offset = aligned_addr - @intFromPtr(stack_space.ptr); + + @memcpy(stack_space[align_offset .. align_offset + ev.args_slice.len], ev.args_slice); + ev.args_slice = stack_space[align_offset .. align_offset + ev.args_slice.len]; + m.args_queue.shrinkRetainingCapacity(m.args_queue.items.len - ev.args_slice.len); m.events_mu.unlock(); @@ -1428,6 +1437,7 @@ test "dispatch" { }; pub const local_events = .{ .update = .{ .handler = update }, + .update_with_struct_arg = .{ .handler = updateWithStructArg }, .calc = .{ .handler = calc }, }; @@ -1439,6 +1449,15 @@ test "dispatch" { global.physics_updates += 1; } + const MyStruct = extern struct { + x: [4]extern struct { x: @Vector(4, f32) } = undefined, + y: [4]extern struct { x: @Vector(4, f32) } = undefined, + }; + fn updateWithStructArg(arg: MyStruct) void { + _ = arg; + global.physics_updates += 1; + } + fn calc() void { global.physics_calc += 1; } @@ -1495,11 +1514,12 @@ test "dispatch" { // injected arguments. modules.sendGlobal(.engine_renderer, .tick, .{}); try testing.expect(usize, 0).eql(global.ticks); - try modules.dispatchInternal(.{}, .{&foo}); + var stack_space: [8 * 1024 * 1024]u8 = undefined; + try modules.dispatchInternal(&stack_space, .{}, .{&foo}); try testing.expect(usize, 2).eql(global.ticks); // TODO: make sendDynamic take an args type to avoid footguns with comptime values, etc. modules.sendGlobalDynamic(@intFromEnum(@as(GE, .tick)), .{}); - try modules.dispatchInternal(.{}, .{&foo}); + try modules.dispatchInternal(&stack_space, .{}, .{&foo}); try testing.expect(usize, 4).eql(global.ticks); // Global events which are not handled by anyone yet can be written as `pub const fooBar = fn() void;` @@ -1509,22 +1529,23 @@ test "dispatch" { // Local events modules.send(.engine_renderer, .update, .{}); - try modules.dispatchInternal(.{}, .{&foo}); + try modules.dispatchInternal(&stack_space, .{}, .{&foo}); try testing.expect(usize, 1).eql(global.renderer_updates); modules.send(.engine_physics, .update, .{}); + modules.send(.engine_physics, .update_with_struct_arg, .{.{}}); modules.sendDynamic( @intFromEnum(@as(M, .engine_physics)), @intFromEnum(@as(LE, .calc)), .{}, ); - try modules.dispatchInternal(.{}, .{&foo}); - try testing.expect(usize, 1).eql(global.physics_updates); + try modules.dispatchInternal(&stack_space, .{}, .{&foo}); + try testing.expect(usize, 2).eql(global.physics_updates); try testing.expect(usize, 1).eql(global.physics_calc); // Local events modules.send(.engine_renderer, .basic_args, .{ @as(u32, 1), @as(u32, 2) }); // TODO: match arguments against fn ArgsTuple, for correctness and type inference modules.send(.engine_renderer, .injected_args, .{ @as(u32, 1), @as(u32, 2) }); - try modules.dispatchInternal(.{}, .{&foo}); + try modules.dispatchInternal(&stack_space, .{}, .{&foo}); try testing.expect(usize, 3).eql(global.basic_args_sum); try testing.expect(usize, 3).eql(foo.injected_args_sum); }