-
-
Notifications
You must be signed in to change notification settings - Fork 47
Remove busy loops #1367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Remove busy loops #1367
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| #include "PathData.hpp" // SoundBlockInfo, SeqPathDataRecord | ||
| #include "../AliveLibAE/Io.hpp" | ||
| #include <assert.h> | ||
| #include <thread> | ||
|
|
||
| struct VagAtr final | ||
| { | ||
|
|
@@ -1346,19 +1347,51 @@ EXPORT void CC SsSetTickMode_4FDC20(s32) | |
| // Stub | ||
| } | ||
|
|
||
| // TODO: Removed 4FDC30 | ||
| EXPORT bool CC SsSeqCalledTbyT_4FDC80_sleep_until_due_time_if_before_max_time(SYS_time_point_t current_time, SYS_time_point_t max_time) | ||
| { | ||
| if (!gSpuVars->sbDisableSeqs()) | ||
| { | ||
| const auto lastTime = gSpuVars->sLastTime(); | ||
| if (lastTime == 0xFFFFFFFF) | ||
| { | ||
| return true; | ||
| } | ||
| else | ||
| { | ||
| const auto nextDueTime = lastTime + 30; | ||
|
|
||
| if ((s32) (SYS_AsU32Ms(max_time) - nextDueTime) < 0) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use static_cast instead of a C cast |
||
| { | ||
| // we won't make it in time, skip | ||
| return false; | ||
| } | ||
|
|
||
| const auto timeToWait = (s32) (nextDueTime - SYS_AsU32Ms(current_time)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use static_cast instead of a C cast |
||
| if (timeToWait <= 0) | ||
| { | ||
| // we're late or right on time, hurry! | ||
| return true; | ||
| } | ||
|
|
||
| std::this_thread::sleep_for(std::chrono::milliseconds{timeToWait}); | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // TODO: Removed 4FDC30 | ||
| EXPORT void CC SsSeqCalledTbyT_4FDC80() | ||
| { | ||
| if (!gSpuVars->sbDisableSeqs()) | ||
| { | ||
| const u32 currentTime = SYS_GetTicks(); | ||
| gSpuVars->sMidiTime() = currentTime; | ||
| // First time or 30 passed? | ||
| if (gSpuVars->sLastTime() == 0xFFFFFFFF || (s32)(currentTime - gSpuVars->sLastTime()) >= 30) | ||
| auto& lastTime = gSpuVars->sLastTime(); | ||
| if (lastTime == 0xFFFFFFFF || (s32) (currentTime - lastTime) >= 30) | ||
| { | ||
| gSpuVars->sLastTime() = currentTime; | ||
| lastTime = currentTime; | ||
| for (s32 i = 0; i < kNumChannels; i++) | ||
| { | ||
| if (gSpuVars->sMidiSeqSongs(i).field_0_seq_data) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,11 @@ HRESULT SDLSoundSystem::Release() | |
|
|
||
| // Stop audio rendering thread | ||
| mRenderAudioThreadQuit = true; | ||
| { | ||
| std::lock_guard g{mAudioRingBufferMutex}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead of 'g' can we at least called it 'lock' or something |
||
| mAudioRingBufferConditionVariable.notify_one(); | ||
| } | ||
|
|
||
| if (mRenderAudioThread && mRenderAudioThread->joinable()) | ||
| { | ||
| mRenderAudioThread->join(); | ||
|
|
@@ -139,6 +144,11 @@ void SDLSoundSystem::AudioCallBack(Uint8* stream, s32 len) | |
| { | ||
| LOG_ERROR("Ring buffer read failure!"); | ||
| } | ||
|
|
||
| { | ||
| std::lock_guard g{mAudioRingBufferMutex}; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto for naming |
||
| mAudioRingBufferConditionVariable.notify_one(); | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
@@ -159,6 +169,12 @@ void SDLSoundSystem::RenderAudioThread() | |
| LOG_ERROR("Ring buffer write failed"); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| std::unique_lock g{mAudioRingBufferMutex}; | ||
| mAudioRingBufferConditionVariable.wait(g, [&] | ||
| { return mRenderAudioThreadQuit || mAudioRingBuffer.getAvailableWrite() > 0; }); | ||
| } | ||
| } | ||
| LOG_INFO("Quit"); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,16 +13,19 @@ | |
| abort(); | ||
| } | ||
|
|
||
| SYS_time_point_t SYS_GetTime() | ||
| { | ||
| return std::chrono::steady_clock::now(); | ||
| } | ||
|
|
||
| u32 SYS_AsU32Ms(SYS_time_point_t t) | ||
| { | ||
| return (u32) std::chrono::duration_cast<std::chrono::milliseconds>(t.time_since_epoch()).count(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. static_cast instead of C cast |
||
| } | ||
|
|
||
| u32 SYS_GetTicks() | ||
| { | ||
| #if USE_SDL2 | ||
| // Using this instead of SDL_GetTicks resolves a weird x64 issue on windows where | ||
| // the tick returned is a lot faster on some machines. | ||
| return static_cast<u32>(SDL_GetPerformanceCounter() / (SDL_GetPerformanceFrequency() / 1000)); | ||
| #else | ||
| return timeGetTime(); | ||
| #endif | ||
| return SYS_AsU32Ms(SYS_GetTime()); | ||
| } | ||
|
|
||
| MessageBoxButton CC Sys_MessageBox(TWindowHandleType windowHandle, const char_type* message, const char_type* title, MessageBoxType type) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_4FDC80 needs to be at the end of the function as exported function names are parsed at runtime for installing hooks into the game when running as a DLL. This name is super verbose too, I would have just left it as SsSeqCalledTbyT_4FDC80 tbh since the waiting/sleeping is an implementation detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to avoid breaking existing SsSeqCalledTbyT_4FDC80 ABI with "max_time" new argument.
(because I understood that number-suffixed functions are meant to be interchangeable with original binaries)
Unless I miss something, I thus can simplify the name but not use the original one.