From bd872ee1c0de006e9140bfa1ee43298c6f50b2af Mon Sep 17 00:00:00 2001 From: Rekai Musuka Date: Fri, 10 Mar 2023 02:02:34 -0600 Subject: [PATCH] fix: drop select atomics in favour of a thread-safe channel --- lib/zba-util | 2 +- src/core/emu.zig | 38 +++++++++++++++++++++++++++++--------- src/main.zig | 15 ++++++++------- src/platform.zig | 32 +++++++++++++++++--------------- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/lib/zba-util b/lib/zba-util index 52ac8d9..d5e66ca 160000 --- a/lib/zba-util +++ b/lib/zba-util @@ -1 +1 @@ -Subproject commit 52ac8d952fe7c722e33a3237e465dcf258076613 +Subproject commit d5e66caf2180324d83ad9be30e887849f5ed74da diff --git a/src/core/emu.zig b/src/core/emu.zig index 215173f..c029943 100644 --- a/src/core/emu.zig +++ b/src/core/emu.zig @@ -5,9 +5,9 @@ const config = @import("../config.zig"); const Scheduler = @import("scheduler.zig").Scheduler; const Arm7tdmi = @import("cpu.zig").Arm7tdmi; const Tracker = @import("../util.zig").FpsTracker; +const TwoWayChannel = @import("zba-util").TwoWayChannel; const Timer = std.time.Timer; -const Atomic = std.atomic.Atomic; /// 4 Cycles in 1 dot const cycles_per_dot = 4; @@ -35,29 +35,40 @@ const RunKind = enum { LimitedFPS, }; -pub fn run(quit: *Atomic(bool), pause: *Atomic(bool), cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: *Tracker) void { +pub fn run(cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: *Tracker, channel: *TwoWayChannel) void { const audio_sync = config.config().guest.audio_sync and !config.config().host.mute; if (audio_sync) log.info("Audio sync enabled", .{}); if (config.config().guest.video_sync) { - inner(.LimitedFPS, audio_sync, quit, pause, cpu, scheduler, tracker); + inner(.LimitedFPS, audio_sync, cpu, scheduler, tracker, channel); } else { - inner(.UnlimitedFPS, audio_sync, quit, pause, cpu, scheduler, tracker); + inner(.UnlimitedFPS, audio_sync, cpu, scheduler, tracker, channel); } } -fn inner(comptime kind: RunKind, audio_sync: bool, quit: *Atomic(bool), pause: *Atomic(bool), cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: ?*Tracker) void { +fn inner(comptime kind: RunKind, audio_sync: bool, cpu: *Arm7tdmi, scheduler: *Scheduler, tracker: ?*Tracker, channel: *TwoWayChannel) void { if (kind == .UnlimitedFPS or kind == .LimitedFPS) { std.debug.assert(tracker != null); log.info("FPS tracking enabled", .{}); } + var paused: bool = false; + switch (kind) { .Unlimited, .UnlimitedFPS => { log.info("Emulation w/out video sync", .{}); - while (!quit.load(.Monotonic)) { - if (pause.load(.Monotonic)) continue; + while (true) { + if (channel.emu.pop()) |e| switch (e) { + .Quit => break, + .Resume => paused = false, + .Pause => { + paused = true; + channel.gui.push(.Paused); + }, + }; + + if (paused) continue; runFrame(scheduler, cpu); audioSync(audio_sync, cpu.bus.apu.stream, &cpu.bus.apu.is_buffer_full); @@ -70,8 +81,17 @@ fn inner(comptime kind: RunKind, audio_sync: bool, quit: *Atomic(bool), pause: * var timer = Timer.start() catch @panic("failed to initalize std.timer.Timer"); var wake_time: u64 = frame_period; - while (!quit.load(.Monotonic)) { - if (pause.load(.Monotonic)) continue; + while (true) { + if (channel.emu.pop()) |e| switch (e) { + .Quit => break, + .Resume => paused = false, + .Pause => { + paused = true; + channel.gui.push(.Paused); + }, + }; + + if (paused) continue; runFrame(scheduler, cpu); const new_wake_time = videoSync(&timer, wake_time); diff --git a/src/main.zig b/src/main.zig index 541ad81..17940ee 100644 --- a/src/main.zig +++ b/src/main.zig @@ -6,6 +6,7 @@ const clap = @import("clap"); const config = @import("config.zig"); const emu = @import("core/emu.zig"); +const TwoWayChannel = @import("zba-util").TwoWayChannel; const Gui = @import("platform.zig").Gui; const Bus = @import("core/Bus.zig"); const Arm7tdmi = @import("core/cpu.zig").Arm7tdmi; @@ -13,7 +14,6 @@ const Scheduler = @import("core/scheduler.zig").Scheduler; const FilePaths = @import("util.zig").FilePaths; const FpsTracker = @import("util.zig").FpsTracker; const Allocator = std.mem.Allocator; -const Atomic = std.atomic.Atomic; const log = std.log.scoped(.Cli); pub const log_level = if (builtin.mode != .Debug) .info else std.log.default_level; @@ -94,7 +94,10 @@ pub fn main() void { var gui = Gui.init(allocator, bus.pak.title, &bus.apu) catch |e| exitln("failed to init gui: {}", .{e}); defer gui.deinit(); - var quit = Atomic(bool).init(false); + var quit = std.atomic.Atomic(bool).init(false); + + var items: [0x100]u8 = undefined; + var channel = TwoWayChannel.init(&items); if (result.args.gdb) { const Server = @import("gdbstub").Server; @@ -117,21 +120,19 @@ pub fn main() void { gui.run(.{ .cpu = &cpu, .scheduler = &scheduler, - .quit = &quit, + .channel = &channel, }) catch |e| exitln("main thread panicked: {}", .{e}); } else { var tracker = FpsTracker.init(); - var pause = Atomic(bool).init(false); - const thread = std.Thread.spawn(.{}, emu.run, .{ &quit, &pause, &cpu, &scheduler, &tracker }) catch |e| exitln("emu thread panicked: {}", .{e}); + const thread = std.Thread.spawn(.{}, emu.run, .{ &cpu, &scheduler, &tracker, &channel }) catch |e| exitln("emu thread panicked: {}", .{e}); defer thread.join(); gui.run(.{ .cpu = &cpu, .scheduler = &scheduler, + .channel = &channel, .tracker = &tracker, - .quit = &quit, - .pause = &pause, }) catch |e| exitln("main thread panicked: {}", .{e}); } } diff --git a/src/platform.zig b/src/platform.zig index 57c395b..30c9e04 100644 --- a/src/platform.zig +++ b/src/platform.zig @@ -11,7 +11,7 @@ const Apu = @import("core/apu.zig").Apu; const Arm7tdmi = @import("core/cpu.zig").Arm7tdmi; const Scheduler = @import("core/scheduler.zig").Scheduler; const FpsTracker = @import("util.zig").FpsTracker; -const RingBuffer = @import("util.zig").RingBuffer; +const TwoWayChannel = @import("zba-util").TwoWayChannel; const gba_width = @import("core/ppu.zig").width; const gba_height = @import("core/ppu.zig").height; @@ -243,8 +243,7 @@ pub const Gui = struct { } const RunOptions = struct { - quit: *std.atomic.Atomic(bool), - pause: ?*std.atomic.Atomic(bool) = null, + channel: *TwoWayChannel, tracker: ?*FpsTracker = null, cpu: *Arm7tdmi, scheduler: *Scheduler, @@ -253,8 +252,7 @@ pub const Gui = struct { pub fn run(self: *Self, opt: RunOptions) !void { const cpu = opt.cpu; const tracker = opt.tracker; - const quit = opt.quit; - const pause = opt.pause; + const channel = opt.channel; const obj_ids = Self.genBufferObjects(); defer gl.deleteBuffers(3, @as(*const [3]c_uint, &obj_ids)); @@ -269,7 +267,10 @@ pub const Gui = struct { emu_loop: while (true) { // `quit` from RunOptions may be modified by the GDBSTUB thread, // so we want to recognize that it may change to `true` and exit the GUI thread - if (quit.load(.Monotonic)) break :emu_loop; + if (channel.gui.pop()) |event| switch (event) { + .Quit => break :emu_loop, + .Paused => @panic("TODO: We want to peek (and then pop if it's .Quit), not always pop"), + }; // Outside of `SDL.SDL_QUIT` below, the DearImgui UI might signal that the program // should exit, in which case we should also handle this @@ -325,17 +326,18 @@ pub const Gui = struct { } } - // If `Gui.run` has been passed with a `pause` atomic, we should - // pause the emulation thread while we access the data there { - // TODO: Is there a nicer way to express this? - if (pause) |val| val.store(true, .Monotonic); - defer if (pause) |val| val.store(false, .Monotonic); + channel.emu.push(.Pause); + defer channel.emu.push(.Resume); + + // Spin Loop until we know that the emu is paused + wait: while (true) switch (channel.gui.pop() orelse continue) { + .Paused => break :wait, + else => |any| std.debug.panic("[Gui/Channel]: Unhandled Event: {}", .{any}), + }; // Add FPS count to the histogram - if (tracker) |t| { - self.state.fps_hist.push(t.value()) catch {}; - } + if (tracker) |t| self.state.fps_hist.push(t.value()) catch {}; // Draw GBA Screen to Texture { @@ -361,7 +363,7 @@ pub const Gui = struct { SDL.SDL_GL_SwapWindow(self.window); } - quit.store(true, .Monotonic); // Signals to emu thread to exit + channel.emu.push(.Quit); } fn glGetProcAddress(ctx: SDL.SDL_GLContext, proc: [:0]const u8) ?*anyopaque {