From 8349005c4b8c4e21cece4adbc7747e0e18e3c282 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sun, 19 Jan 2020 23:52:47 -0500 Subject: [PATCH 1/7] Adds CRTMachine::run_until, which will run until a condition is true. I want to get to being able to say "run until the beam is 60% of the way down", "run until a new packet of audio has been delivered", etc. --- Machines/CRTMachine.hpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Machines/CRTMachine.hpp b/Machines/CRTMachine.hpp index 64be17500..8d6d6a99c 100644 --- a/Machines/CRTMachine.hpp +++ b/Machines/CRTMachine.hpp @@ -45,12 +45,20 @@ class Machine { virtual std::string debug_type() { return ""; } /// Runs the machine for @c duration seconds. - virtual void run_for(Time::Seconds duration) { + void run_for(Time::Seconds duration) { const double cycles = (duration * clock_rate_) + clock_conversion_error_; clock_conversion_error_ = std::fmod(cycles, 1.0); run_for(Cycles(static_cast(cycles))); } + /// Runs for the machine for at least @c duration seconds, and then until @c condition is true. + void run_until(Time::Seconds minimum_duration, std::function condition) { + run_for(minimum_duration); + while(!condition()) { + run_for(0.002); + } + } + protected: /// Runs the machine for @c cycles. virtual void run_for(const Cycles cycles) = 0; From cb61e8486826b10c1c5b7939a6aae4664d8d6dd9 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Jan 2020 12:12:23 -0500 Subject: [PATCH 2/7] Starts building out higher-level `run_until` functionality. Specifically: you can now run until the next set of speaker samples has been delivered. --- .../Implementation/MultiSpeaker.cpp | 2 +- Machines/CRTMachine.hpp | 46 +++++++++++++++++-- .../Speaker/Implementation/LowpassSpeaker.hpp | 4 +- Outputs/Speaker/Speaker.hpp | 7 +++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/Analyser/Dynamic/MultiMachine/Implementation/MultiSpeaker.cpp b/Analyser/Dynamic/MultiMachine/Implementation/MultiSpeaker.cpp index 26ec1d4de..c5a8fa724 100644 --- a/Analyser/Dynamic/MultiMachine/Implementation/MultiSpeaker.cpp +++ b/Analyser/Dynamic/MultiMachine/Implementation/MultiSpeaker.cpp @@ -53,7 +53,7 @@ void MultiSpeaker::speaker_did_complete_samples(Speaker *speaker, const std::vec std::lock_guard lock_guard(front_speaker_mutex_); if(speaker != front_speaker_) return; } - delegate_->speaker_did_complete_samples(this, buffer); + did_complete_samples(this, buffer); } void MultiSpeaker::speaker_did_change_input_clock(Speaker *speaker) { diff --git a/Machines/CRTMachine.hpp b/Machines/CRTMachine.hpp index 8d6d6a99c..ff6aa7800 100644 --- a/Machines/CRTMachine.hpp +++ b/Machines/CRTMachine.hpp @@ -45,17 +45,57 @@ class Machine { virtual std::string debug_type() { return ""; } /// Runs the machine for @c duration seconds. - void run_for(Time::Seconds duration) { + virtual void run_for(Time::Seconds duration) { const double cycles = (duration * clock_rate_) + clock_conversion_error_; clock_conversion_error_ = std::fmod(cycles, 1.0); run_for(Cycles(static_cast(cycles))); } - /// Runs for the machine for at least @c duration seconds, and then until @c condition is true. - void run_until(Time::Seconds minimum_duration, std::function condition) { + /*! + Runs for the machine for at least @c duration seconds, and then until @c condition is true. + + @returns The amount of time run for. + */ + Time::Seconds run_until(Time::Seconds minimum_duration, std::function condition) { + Time::Seconds total_runtime = minimum_duration; run_for(minimum_duration); while(!condition()) { + // Advance in increments of one 500th of a second until the condition + // is true; that's 1/10th of a 50Hz frame, but more like 1/8.33 of a + // 60Hz frame. Though most machines aren't exactly 50Hz or 60Hz, and some + // are arbitrary other refresh rates. So those observations are merely + // for scale. run_for(0.002); + total_runtime += 0.002; + } + return total_runtime; + } + + enum class MachineEvent { + /// At least one new packet of audio has been delivered to the spaker's delegate. + NewSpeakerSamplesGenerated + }; + + /*! + Runs for at least @c duration seconds, and then until @c event has occurred at least once since this + call to @c run_until_event. + + @returns The amount of time run for. + */ + Time::Seconds run_until(Time::Seconds minimum_duration, MachineEvent event) { + switch(event) { + case MachineEvent::NewSpeakerSamplesGenerated: { + const auto speaker = get_speaker(); + if(!speaker) { + run_for(minimum_duration); + return minimum_duration; + } + + const int sample_sets = speaker->completed_sample_sets(); + return run_until(minimum_duration, [sample_sets, speaker]() { + return speaker->completed_sample_sets() != sample_sets; + }); + } break; } } diff --git a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp index 38c9fed23..ced763a56 100644 --- a/Outputs/Speaker/Implementation/LowpassSpeaker.hpp +++ b/Outputs/Speaker/Implementation/LowpassSpeaker.hpp @@ -134,7 +134,7 @@ template class LowpassSpeaker: public Speaker { // announce to delegate if full if(output_buffer_pointer_ == output_buffer_.size()) { output_buffer_pointer_ = 0; - delegate_->speaker_did_complete_samples(this, output_buffer_); + did_complete_samples(this, output_buffer_); } cycles_remaining -= cycles_to_read; @@ -159,7 +159,7 @@ template class LowpassSpeaker: public Speaker { // Announce to delegate if full. if(output_buffer_pointer_ == output_buffer_.size()) { output_buffer_pointer_ = 0; - delegate_->speaker_did_complete_samples(this, output_buffer_); + did_complete_samples(this, output_buffer_); } // If the next loop around is going to reuse some of the samples just collected, use a memmove to diff --git a/Outputs/Speaker/Speaker.hpp b/Outputs/Speaker/Speaker.hpp index 11b06bfde..7c5a31437 100644 --- a/Outputs/Speaker/Speaker.hpp +++ b/Outputs/Speaker/Speaker.hpp @@ -26,6 +26,8 @@ class Speaker { virtual float get_ideal_clock_rate_in_range(float minimum, float maximum) = 0; virtual void set_output_rate(float cycles_per_second, int buffer_size) = 0; + int completed_sample_sets() { return completed_sample_sets_; } + struct Delegate { virtual void speaker_did_complete_samples(Speaker *speaker, const std::vector &buffer) = 0; virtual void speaker_did_change_input_clock(Speaker *speaker) {} @@ -35,7 +37,12 @@ class Speaker { } protected: + void did_complete_samples(Speaker *speaker, const std::vector &buffer) { + ++completed_sample_sets_; + delegate_->speaker_did_complete_samples(this, buffer); + } Delegate *delegate_ = nullptr; + int completed_sample_sets_ = 0; }; } From 8adb2283b58252c30726b49d8171f3264f63d614 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Jan 2020 13:38:46 -0500 Subject: [PATCH 3/7] Rewrites the best-effort updater to try to get better thread affinity. --- Concurrency/BestEffortUpdater.cpp | 111 +++++++++++------- Concurrency/BestEffortUpdater.hpp | 25 +++- .../xcschemes/Clock Signal.xcscheme | 2 +- 3 files changed, 90 insertions(+), 48 deletions(-) diff --git a/Concurrency/BestEffortUpdater.cpp b/Concurrency/BestEffortUpdater.cpp index 3d452a081..17c2b942c 100644 --- a/Concurrency/BestEffortUpdater.cpp +++ b/Concurrency/BestEffortUpdater.cpp @@ -12,61 +12,90 @@ using namespace Concurrency; -BestEffortUpdater::BestEffortUpdater() { - // ATOMIC_FLAG_INIT isn't necessarily safe to use, so establish default state by other means. - update_is_ongoing_.clear(); -} +BestEffortUpdater::BestEffortUpdater() : + update_thread_([this]() { + this->update_loop(); + }) {} BestEffortUpdater::~BestEffortUpdater() { - // Don't allow further deconstruction until the task queue is stopped. + // Sever the delegate now, as soon as possible, then wait for any + // pending tasks to finish. + set_delegate(nullptr); flush(); + + // Wind up the update thread. + should_quit_ = true; + update(); + update_thread_.join(); } void BestEffortUpdater::update() { - // Perform an update only if one is not currently ongoing. - if(!update_is_ongoing_.test_and_set()) { - async_task_queue_.enqueue([this]() { - // Get time now using the highest-resolution clock provided by the implementation, and determine - // the duration since the last time this section was entered. - const std::chrono::time_point now = std::chrono::high_resolution_clock::now(); - const auto elapsed = now - previous_time_point_; - previous_time_point_ = now; + // Bump the requested target time and set the update requested flag. + { + std::lock_guard lock(update_mutex_); + has_skipped_ = update_requested_; + update_requested_ = true; + target_time_ = std::chrono::high_resolution_clock::now(); + } + update_condition_.notify_one(); +} - if(has_previous_time_point_) { - // If the duration is valid, convert it to integer cycles, maintaining a rolling error and call the delegate - // if there is one. Proceed only if the number of cycles is positive, and cap it to the per-second maximum as - // it's possible this is an adjustable clock so be ready to swallow unexpected adjustments. - const int64_t integer_duration = std::chrono::duration_cast(elapsed).count(); - if(integer_duration > 0) { - if(delegate_) { - // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any - // brief system interruption. - const double duration = std::min(static_cast(integer_duration) / 1e9, 0.2); - delegate_->update(this, duration, has_skipped_); - } - has_skipped_ = false; - } - } else { - has_previous_time_point_ = true; +void BestEffortUpdater::update_loop() { + while(true) { + std::unique_lock lock(update_mutex_); + is_updating_ = false; + + // Wait to be signalled. + update_condition_.wait(lock, [this]() -> bool { + return update_requested_; + }); + + // Possibly this signalling really means 'quit'. + if(should_quit_) return; + + // Note update started, crib the target time. + auto target_time = target_time_; + update_requested_ = false; + + // If this was actually the first update request, silently swallow it. + if(!has_previous_time_point_) { + has_previous_time_point_ = true; + previous_time_point_ = target_time; + continue; + } + + // Release the lock on requesting new updates. + is_updating_ = true; + lock.unlock(); + + // Calculate period from previous time to now. + const auto elapsed = target_time - previous_time_point_; + previous_time_point_ = target_time; + + // Invoke the delegate, if supplied, in order to run. + const int64_t integer_duration = std::chrono::duration_cast(elapsed).count(); + if(integer_duration > 0) { + const auto delegate = delegate_.load(); + if(delegate) { + // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any + // brief system interruption. + const double duration = std::min(double(integer_duration) / 1e9, 0.2); + delegate->update(this, duration, has_skipped_); + has_skipped_ = false; } - - // Allow furthers updates to occur. - update_is_ongoing_.clear(); - }); - } else { - async_task_queue_.enqueue([this]() { - has_skipped_ = true; - }); + } } } void BestEffortUpdater::flush() { - async_task_queue_.flush(); + // Spin lock; this is allowed to be slow. + while(true) { + std::lock_guard lock(update_mutex_); + if(!is_updating_) return; + } } void BestEffortUpdater::set_delegate(Delegate *const delegate) { - async_task_queue_.enqueue([this, delegate]() { - delegate_ = delegate; - }); + delegate_.store(delegate); } diff --git a/Concurrency/BestEffortUpdater.hpp b/Concurrency/BestEffortUpdater.hpp index 6979c6eaa..38688d415 100644 --- a/Concurrency/BestEffortUpdater.hpp +++ b/Concurrency/BestEffortUpdater.hpp @@ -11,8 +11,10 @@ #include #include +#include +#include +#include -#include "AsyncTaskQueue.hpp" #include "../ClockReceiver/TimeTypes.hpp" namespace Concurrency { @@ -43,18 +45,29 @@ class BestEffortUpdater { */ void update(); - /// Blocks until any ongoing update is complete. + /// Blocks until any ongoing update is complete; may spin. void flush(); private: - std::atomic_flag update_is_ongoing_; - AsyncTaskQueue async_task_queue_; + std::atomic should_quit_; + std::atomic is_updating_; + + std::chrono::time_point target_time_; + bool update_requested_; + std::mutex update_mutex_; + std::condition_variable update_condition_; std::chrono::time_point previous_time_point_; bool has_previous_time_point_ = false; - bool has_skipped_ = false; + std::atomic has_skipped_ = false; - Delegate *delegate_ = nullptr; + std::atomicdelegate_ = nullptr; + + void update_loop(); + + // This is deliberately at the bottom, to ensure it constructs after the various + // mutexs, conditions, etc, that it'll depend upon. + std::thread update_thread_; }; } diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 47f9c7286..1465a4f62 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ Date: Mon, 20 Jan 2020 16:08:21 -0500 Subject: [PATCH 4/7] Switches to accepting a bit mask of events to run_until. --- Machines/CRTMachine.hpp | 35 ++++++++++--------- .../xcschemes/Clock Signal.xcscheme | 2 +- Outputs/Speaker/Speaker.hpp | 2 +- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/Machines/CRTMachine.hpp b/Machines/CRTMachine.hpp index ff6aa7800..db100c99c 100644 --- a/Machines/CRTMachine.hpp +++ b/Machines/CRTMachine.hpp @@ -71,32 +71,33 @@ class Machine { return total_runtime; } - enum class MachineEvent { + enum MachineEvent: int { /// At least one new packet of audio has been delivered to the spaker's delegate. - NewSpeakerSamplesGenerated + NewSpeakerSamplesGenerated = 1 << 0 }; /*! - Runs for at least @c duration seconds, and then until @c event has occurred at least once since this + Runs for at least @c duration seconds, and then every one of the @c events has occurred at least once since this call to @c run_until_event. + @param events A bitmask comprised of @c MachineEvent flags. @returns The amount of time run for. */ - Time::Seconds run_until(Time::Seconds minimum_duration, MachineEvent event) { - switch(event) { - case MachineEvent::NewSpeakerSamplesGenerated: { - const auto speaker = get_speaker(); - if(!speaker) { - run_for(minimum_duration); - return minimum_duration; - } - - const int sample_sets = speaker->completed_sample_sets(); - return run_until(minimum_duration, [sample_sets, speaker]() { - return speaker->completed_sample_sets() != sample_sets; - }); - } break; + Time::Seconds run_until(Time::Seconds minimum_duration, int events) { + // Tie up a wait-for-samples, if requested. + const Outputs::Speaker::Speaker *speaker = nullptr; + int sample_sets = 0; + if(events & MachineEvent::NewSpeakerSamplesGenerated) { + speaker = get_speaker(); + if(!speaker) events &= ~MachineEvent::NewSpeakerSamplesGenerated; + sample_sets = speaker->completed_sample_sets(); } + + // Run until all requested events are satisfied. + return run_until(minimum_duration, [=]() { + return + (!(events & MachineEvent::NewSpeakerSamplesGenerated) || (sample_sets != speaker->completed_sample_sets())); + }); } protected: diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 1465a4f62..47f9c7286 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ &buffer) = 0; From 4de121142bdf6363476a1ad92673ff9456733d1a Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Jan 2020 16:21:53 -0500 Subject: [PATCH 5/7] Adds a flags parameter to the BestEffortUpdater delegate. On the Cocoa side, cuts Swift out of the update loop, as that seems merely to add code. --- Concurrency/BestEffortUpdater.cpp | 2 +- Concurrency/BestEffortUpdater.hpp | 2 +- .../xcschemes/Clock Signal.xcscheme | 2 +- .../Documents/MachineDocument.swift | 6 ++-- .../Updater/CSBestEffortUpdater.h | 12 ++----- .../Updater/CSBestEffortUpdater.mm | 35 ++++--------------- OSBindings/SDL/main.cpp | 4 +-- 7 files changed, 15 insertions(+), 48 deletions(-) diff --git a/Concurrency/BestEffortUpdater.cpp b/Concurrency/BestEffortUpdater.cpp index 17c2b942c..16aa97199 100644 --- a/Concurrency/BestEffortUpdater.cpp +++ b/Concurrency/BestEffortUpdater.cpp @@ -80,7 +80,7 @@ void BestEffortUpdater::update_loop() { // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any // brief system interruption. const double duration = std::min(double(integer_duration) / 1e9, 0.2); - delegate->update(this, duration, has_skipped_); + delegate->update(this, duration, has_skipped_, 0); has_skipped_ = false; } } diff --git a/Concurrency/BestEffortUpdater.hpp b/Concurrency/BestEffortUpdater.hpp index 38688d415..f46490b85 100644 --- a/Concurrency/BestEffortUpdater.hpp +++ b/Concurrency/BestEffortUpdater.hpp @@ -33,7 +33,7 @@ class BestEffortUpdater { /// A delegate receives timing cues. struct Delegate { - virtual void update(BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update) = 0; + virtual void update(BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) = 0; }; /// Sets the current delegate. diff --git a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme index 47f9c7286..1465a4f62 100644 --- a/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme +++ b/OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme @@ -67,7 +67,7 @@ #import -@class CSBestEffortUpdater; - -@protocol CSBestEffortUpdaterDelegate - -- (void)bestEffortUpdater:(CSBestEffortUpdater *)bestEffortUpdater runForInterval:(NSTimeInterval)interval didSkipPreviousUpdate:(BOOL)didSkipPreviousUpdate; - -@end - +#import "CSMachine.h" @interface CSBestEffortUpdater : NSObject -@property (nonatomic, weak) id delegate; - - (void)update; - (void)flush; +- (void)setMachine:(CSMachine *)machine; @end diff --git a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm index bc22986db..5b5fd357b 100644 --- a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm +++ b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm @@ -11,38 +11,26 @@ #include "BestEffortUpdater.hpp" struct UpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { - __weak id delegate; - NSLock *delegateLock; + __weak CSMachine *machine; - void update(Concurrency::BestEffortUpdater *updater, Time::Seconds cycles, bool did_skip_previous_update) { - [delegateLock lock]; - __weak id delegateCopy = delegate; - [delegateLock unlock]; - - [delegateCopy bestEffortUpdater:nil runForInterval:(NSTimeInterval)cycles didSkipPreviousUpdate:did_skip_previous_update]; + void update(Concurrency::BestEffortUpdater *updater, Time::Seconds seconds, bool did_skip_previous_update, int flags) { + [machine runForInterval:seconds]; } }; @implementation CSBestEffortUpdater { Concurrency::BestEffortUpdater _updater; UpdaterDelegate _updaterDelegate; - NSLock *_delegateLock; } - (instancetype)init { self = [super init]; if(self) { - _delegateLock = [[NSLock alloc] init]; - _updaterDelegate.delegateLock = _delegateLock; _updater.set_delegate(&_updaterDelegate); } return self; } -//- (void)dealloc { -// _updater.flush(); -//} - - (void)update { _updater.update(); } @@ -51,20 +39,9 @@ struct UpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { _updater.flush(); } -- (void)setDelegate:(id)delegate { - [_delegateLock lock]; - _updaterDelegate.delegate = delegate; - [_delegateLock unlock]; -} - -- (id)delegate { - id delegate; - - [_delegateLock lock]; - delegate = _updaterDelegate.delegate; - [_delegateLock unlock]; - - return delegate; +- (void)setMachine:(CSMachine *)machine { + _updater.flush(); + _updaterDelegate.machine = machine; } @end diff --git a/OSBindings/SDL/main.cpp b/OSBindings/SDL/main.cpp index 4a44d0789..74710fad8 100644 --- a/OSBindings/SDL/main.cpp +++ b/OSBindings/SDL/main.cpp @@ -34,8 +34,8 @@ namespace { struct BestEffortUpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { - void update(Concurrency::BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update) override { - machine->crt_machine()->run_for(duration); + void update(Concurrency::BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) override { + machine->crt_machine()->run_until(duration); } Machine::DynamicMachine *machine; From 290db67f093d04bd30962bc746c22238029f7ce8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Jan 2020 17:09:01 -0500 Subject: [PATCH 6/7] Adds a forward route for event flags. Doesn't yet account for extra time expended. --- Concurrency/BestEffortUpdater.cpp | 7 +++++-- Concurrency/BestEffortUpdater.hpp | 3 ++- .../Mac/Clock Signal/Documents/MachineDocument.swift | 8 -------- OSBindings/Mac/Clock Signal/Machine/CSMachine.h | 2 +- OSBindings/Mac/Clock Signal/Machine/CSMachine.mm | 4 ++-- .../Mac/Clock Signal/Updater/CSBestEffortUpdater.mm | 2 +- OSBindings/SDL/main.cpp | 2 +- 7 files changed, 12 insertions(+), 16 deletions(-) diff --git a/Concurrency/BestEffortUpdater.cpp b/Concurrency/BestEffortUpdater.cpp index 16aa97199..fe68c58f8 100644 --- a/Concurrency/BestEffortUpdater.cpp +++ b/Concurrency/BestEffortUpdater.cpp @@ -29,12 +29,13 @@ BestEffortUpdater::~BestEffortUpdater() { update_thread_.join(); } -void BestEffortUpdater::update() { +void BestEffortUpdater::update(int flags) { // Bump the requested target time and set the update requested flag. { std::lock_guard lock(update_mutex_); has_skipped_ = update_requested_; update_requested_ = true; + flags_ |= flags; target_time_ = std::chrono::high_resolution_clock::now(); } update_condition_.notify_one(); @@ -66,6 +67,8 @@ void BestEffortUpdater::update_loop() { // Release the lock on requesting new updates. is_updating_ = true; + const int flags = flags_; + flags_ = 0; lock.unlock(); // Calculate period from previous time to now. @@ -80,7 +83,7 @@ void BestEffortUpdater::update_loop() { // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any // brief system interruption. const double duration = std::min(double(integer_duration) / 1e9, 0.2); - delegate->update(this, duration, has_skipped_, 0); + delegate->update(this, duration, has_skipped_, flags); has_skipped_ = false; } } diff --git a/Concurrency/BestEffortUpdater.hpp b/Concurrency/BestEffortUpdater.hpp index f46490b85..8626d283f 100644 --- a/Concurrency/BestEffortUpdater.hpp +++ b/Concurrency/BestEffortUpdater.hpp @@ -43,7 +43,7 @@ class BestEffortUpdater { If the delegate is not currently in the process of an `update` call, calls it now to catch up to the current time. The call is asynchronous; this method will return immediately. */ - void update(); + void update(int flags = 0); /// Blocks until any ongoing update is complete; may spin. void flush(); @@ -53,6 +53,7 @@ class BestEffortUpdater { std::atomic is_updating_; std::chrono::time_point target_time_; + int flags_ = 0; bool update_requested_; std::mutex update_mutex_; std::condition_variable update_condition_; diff --git a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift index 79dd8e6f7..098a3c230 100644 --- a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift +++ b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift @@ -269,14 +269,6 @@ class MachineDocument: } } - /// Responds to CSBestEffortUpdaterDelegate update message by running the machine. - final func bestEffortUpdater(_ bestEffortUpdater: CSBestEffortUpdater!, runForInterval duration: TimeInterval, didSkipPreviousUpdate: Bool) { - if let machine = self.machine, actionLock.try() { - machine.run(forInterval: duration) - actionLock.unlock() - } - } - // MARK: - Pasteboard Forwarding. /// Forwards any text currently on the pasteboard into the active machine. diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h index 660bd3bf8..d2d2b1ebb 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h @@ -57,7 +57,7 @@ typedef NS_ENUM(NSInteger, CSMachineKeyboardInputMode) { */ - (nullable instancetype)initWithAnalyser:(nonnull CSStaticAnalyser *)result missingROMs:(nullable inout NSMutableArray *)missingROMs NS_DESIGNATED_INITIALIZER; -- (void)runForInterval:(NSTimeInterval)interval; +- (void)runForInterval:(NSTimeInterval)interval untilEvent:(int)events; - (float)idealSamplingRateFromRange:(NSRange)range; - (void)setAudioSamplingRate:(float)samplingRate bufferSize:(NSUInteger)bufferSize; diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index 9f36cd417..c32e52f81 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -259,7 +259,7 @@ struct ActivityObserver: public Activity::Observer { } } -- (void)runForInterval:(NSTimeInterval)interval { +- (void)runForInterval:(NSTimeInterval)interval untilEvent:(int)events { @synchronized(self) { if(_joystickMachine && _joystickManager) { [_joystickManager update]; @@ -309,7 +309,7 @@ struct ActivityObserver: public Activity::Observer { } } } - _machine->crt_machine()->run_for(interval); + _machine->crt_machine()->run_until(interval, events); } } diff --git a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm index 5b5fd357b..3c4142fbf 100644 --- a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm +++ b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm @@ -14,7 +14,7 @@ struct UpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { __weak CSMachine *machine; void update(Concurrency::BestEffortUpdater *updater, Time::Seconds seconds, bool did_skip_previous_update, int flags) { - [machine runForInterval:seconds]; + [machine runForInterval:seconds untilEvent:flags]; } }; diff --git a/OSBindings/SDL/main.cpp b/OSBindings/SDL/main.cpp index 74710fad8..dab9b9159 100644 --- a/OSBindings/SDL/main.cpp +++ b/OSBindings/SDL/main.cpp @@ -35,7 +35,7 @@ namespace { struct BestEffortUpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { void update(Concurrency::BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) override { - machine->crt_machine()->run_until(duration); + machine->crt_machine()->run_until(duration, flags); } Machine::DynamicMachine *machine; From 3aa2c297a2db12912c562840d1e9435725b09231 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 20 Jan 2020 17:38:25 -0500 Subject: [PATCH 7/7] Adds feedback to the best-effort updater; enables the Cocoa port for audio event requests. --- Concurrency/BestEffortUpdater.cpp | 26 ++++++++----------- Concurrency/BestEffortUpdater.hpp | 12 ++++++--- .../Documents/MachineDocument.swift | 2 +- .../Mac/Clock Signal/Machine/CSMachine.h | 2 +- .../Mac/Clock Signal/Machine/CSMachine.mm | 4 +-- .../Updater/CSBestEffortUpdater.h | 7 +++++ .../Updater/CSBestEffortUpdater.mm | 8 ++++-- OSBindings/SDL/main.cpp | 4 +-- 8 files changed, 39 insertions(+), 26 deletions(-) diff --git a/Concurrency/BestEffortUpdater.cpp b/Concurrency/BestEffortUpdater.cpp index fe68c58f8..6eb3635a2 100644 --- a/Concurrency/BestEffortUpdater.cpp +++ b/Concurrency/BestEffortUpdater.cpp @@ -36,7 +36,7 @@ void BestEffortUpdater::update(int flags) { has_skipped_ = update_requested_; update_requested_ = true; flags_ |= flags; - target_time_ = std::chrono::high_resolution_clock::now(); + target_time_ = std::chrono::high_resolution_clock::now().time_since_epoch().count(); } update_condition_.notify_one(); } @@ -71,21 +71,17 @@ void BestEffortUpdater::update_loop() { flags_ = 0; lock.unlock(); - // Calculate period from previous time to now. - const auto elapsed = target_time - previous_time_point_; - previous_time_point_ = target_time; - // Invoke the delegate, if supplied, in order to run. - const int64_t integer_duration = std::chrono::duration_cast(elapsed).count(); - if(integer_duration > 0) { - const auto delegate = delegate_.load(); - if(delegate) { - // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any - // brief system interruption. - const double duration = std::min(double(integer_duration) / 1e9, 0.2); - delegate->update(this, duration, has_skipped_, flags); - has_skipped_ = false; - } + const int64_t integer_duration = std::max(target_time - previous_time_point_, int64_t(0)); + const auto delegate = delegate_.load(); + if(delegate) { + // Cap running at 1/5th of a second, to avoid doing a huge amount of work after any + // brief system interruption. + const double duration = std::min(double(integer_duration) / 1e9, 0.2); + const double elapsed_duraation = delegate->update(this, duration, has_skipped_, flags); + + previous_time_point_ += int64_t(elapsed_duraation * 1e9); + has_skipped_ = false; } } } diff --git a/Concurrency/BestEffortUpdater.hpp b/Concurrency/BestEffortUpdater.hpp index 8626d283f..edf92edb7 100644 --- a/Concurrency/BestEffortUpdater.hpp +++ b/Concurrency/BestEffortUpdater.hpp @@ -33,7 +33,13 @@ class BestEffortUpdater { /// A delegate receives timing cues. struct Delegate { - virtual void update(BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) = 0; + /*! + Instructs the delegate to run for at least @c duration, providing hints as to whether multiple updates were requested before the previous had completed + (as @c did_skip_previous_update) and providing the union of any flags supplied to @c update. + + @returns The amount of time actually run for. + */ + virtual Time::Seconds update(BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) = 0; }; /// Sets the current delegate. @@ -52,13 +58,13 @@ class BestEffortUpdater { std::atomic should_quit_; std::atomic is_updating_; - std::chrono::time_point target_time_; + int64_t target_time_; int flags_ = 0; bool update_requested_; std::mutex update_mutex_; std::condition_variable update_condition_; - std::chrono::time_point previous_time_point_; + decltype(target_time_) previous_time_point_; bool has_previous_time_point_ = false; std::atomic has_skipped_ = false; diff --git a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift index 098a3c230..da82b2e3d 100644 --- a/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift +++ b/OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift @@ -243,7 +243,7 @@ class MachineDocument: /// Responds to the CSAudioQueueDelegate dry-queue warning message by requesting a machine update. final func audioQueueIsRunningDry(_ audioQueue: CSAudioQueue) { bestEffortLock.lock() - bestEffortUpdater?.update() + bestEffortUpdater?.update(with: .audioNeeded) bestEffortLock.unlock() } diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h index d2d2b1ebb..83f6d072b 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.h +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.h @@ -57,7 +57,7 @@ typedef NS_ENUM(NSInteger, CSMachineKeyboardInputMode) { */ - (nullable instancetype)initWithAnalyser:(nonnull CSStaticAnalyser *)result missingROMs:(nullable inout NSMutableArray *)missingROMs NS_DESIGNATED_INITIALIZER; -- (void)runForInterval:(NSTimeInterval)interval untilEvent:(int)events; +- (NSTimeInterval)runForInterval:(NSTimeInterval)interval untilEvent:(int)events; - (float)idealSamplingRateFromRange:(NSRange)range; - (void)setAudioSamplingRate:(float)samplingRate bufferSize:(NSUInteger)bufferSize; diff --git a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm index c32e52f81..dbde81399 100644 --- a/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm +++ b/OSBindings/Mac/Clock Signal/Machine/CSMachine.mm @@ -259,7 +259,7 @@ struct ActivityObserver: public Activity::Observer { } } -- (void)runForInterval:(NSTimeInterval)interval untilEvent:(int)events { +- (NSTimeInterval)runForInterval:(NSTimeInterval)interval untilEvent:(int)events { @synchronized(self) { if(_joystickMachine && _joystickManager) { [_joystickManager update]; @@ -309,7 +309,7 @@ struct ActivityObserver: public Activity::Observer { } } } - _machine->crt_machine()->run_until(interval, events); + return _machine->crt_machine()->run_until(interval, events); } } diff --git a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.h b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.h index 24e22c088..970104107 100644 --- a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.h +++ b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.h @@ -11,9 +11,16 @@ #import "CSMachine.h" +// The following is coupled to the definitions in CRTMachine.hpp, but exposed here +// for the benefit of Swift. +typedef NS_ENUM(NSInteger, CSBestEffortUpdaterEvent) { + CSBestEffortUpdaterEventAudioNeeded = 1 << 0 +}; + @interface CSBestEffortUpdater : NSObject - (void)update; +- (void)updateWithEvent:(CSBestEffortUpdaterEvent)event; - (void)flush; - (void)setMachine:(CSMachine *)machine; diff --git a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm index 3c4142fbf..a0ba3e21f 100644 --- a/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm +++ b/OSBindings/Mac/Clock Signal/Updater/CSBestEffortUpdater.mm @@ -13,8 +13,8 @@ struct UpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { __weak CSMachine *machine; - void update(Concurrency::BestEffortUpdater *updater, Time::Seconds seconds, bool did_skip_previous_update, int flags) { - [machine runForInterval:seconds untilEvent:flags]; + Time::Seconds update(Concurrency::BestEffortUpdater *updater, Time::Seconds seconds, bool did_skip_previous_update, int flags) final { + return [machine runForInterval:seconds untilEvent:flags]; } }; @@ -35,6 +35,10 @@ struct UpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { _updater.update(); } +- (void)updateWithEvent:(CSBestEffortUpdaterEvent)event { + _updater.update((int)event); +} + - (void)flush { _updater.flush(); } diff --git a/OSBindings/SDL/main.cpp b/OSBindings/SDL/main.cpp index dab9b9159..6a70d98dd 100644 --- a/OSBindings/SDL/main.cpp +++ b/OSBindings/SDL/main.cpp @@ -34,8 +34,8 @@ namespace { struct BestEffortUpdaterDelegate: public Concurrency::BestEffortUpdater::Delegate { - void update(Concurrency::BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) override { - machine->crt_machine()->run_until(duration, flags); + Time::Seconds update(Concurrency::BestEffortUpdater *updater, Time::Seconds duration, bool did_skip_previous_update, int flags) override { + return machine->crt_machine()->run_until(duration, flags); } Machine::DynamicMachine *machine;