diff --git a/frontend/drivers/platform_apple.c b/frontend/drivers/platform_apple.c index ed64148dc1..802179e8be 100644 --- a/frontend/drivers/platform_apple.c +++ b/frontend/drivers/platform_apple.c @@ -30,6 +30,14 @@ //#define DEBUG_RUNLOOP +#ifdef DEBUG_RUNLOOP +#include "../../retroarch_logger.h" + +#define CF_LOG(...) RARCH_LOG(__VA_ARGS__) +#else +#define CF_LOG(...) +#endif + void apple_start_iteration(void); void apple_stop_iteration(void); @@ -48,101 +56,95 @@ static int do_ra_iteration(void) static void do_iteration(CFRunLoopObserverRef observer, CFRunLoopActivity activity, void *info) { -#ifdef DEBUG_RUNLOOP - if (activity & kCFRunLoopEntry) + switch (activity) { - RARCH_LOG("RUNLOOP ENTRY, frame: %d.\n", g_runloop.frames.video.count); + case kCFRunLoopEntry: + CF_LOG("RUNLOOP ENTRY, frame: %d.\n", g_runloop.frames.video.count); + break; + case kCFRunLoopBeforeTimers: + CF_LOG("RUNLOOP BEFORE TIMERS, frame: %d.\n", g_runloop.frames.video.count); + break; + case kCFRunLoopAfterWaiting: + CF_LOG("RUNLOOP AFTER WAITING, frame: %d.\n", g_runloop.frames.video.count); + break; + case kCFRunLoopBeforeWaiting: + { + CF_LOG("RUNLOOP BEFORE WAITING, frame: %d.\n", g_runloop.frames.video.count); + int ret = do_ra_iteration(); + if (ret == -1) + return; + + CFRunLoopWakeUp(CFRunLoopGetMain()); + + /* TODO/FIXME + I am almost positive that this is not necessary and is actually a + bad thing. + + 1st. Why it is bad thing. + + This wakes up the main event loop immediately and the main loop + has only one observer, which is this function. In other words, + this causes the function to be called immediately. I did an + experiment where I saved the time before calling this and then + reported the difference between it and the start of + do_iteration, and as expected it was about 0. As a result, when + we remove this, idle performance (i.e. displaying the RetroArch + menu) is 0% CPU as desired. + + 2nd. Why it is not necessary. + + The event loop will wake up itself when there is input to the + process. This includes touch events, keyboard, bluetooth, + etc. Thus, it will be woken up and without any intervention so + that we can process that event. + + Nota bene. Why this analysis might be wrong (and what to do about it). + + If RA is not idle and is running a core, then I believe it is + designed to expect to be called in a busy loop like this because + it implements its own frame timer to ensure that the emulation + simulation isn't too fast. In that case, this change would only + allow emulation to run when there was input, which would make + all games turn-based. :) + + There are two good ways to fix this and still have the desired + 0% CPU idle behavior. + + Approach 1: Change main_entry_decide from returning a boolean + (two-values) that are interpreted as CONTINUE and QUIT. Into + returning a char-sized enum with three values that are + interpreted as QUIT, WAIT, and AGAIN, such that QUIT calls + main_exit, WAIT doesn't wake up the loop, and AGAIN does. It + would then return AGAIN when a core was active. An ugly way to + get the same effect is to look have this code just look at + g_runloop.is_menu and use the WAIT behavior in that case. + + Approach 2: Instead of signalling outside of RA whether a core + is running, instead externalize the frame time that is inside + retroarch. change main_entry_decide to return a value in + [-1,MAX_INT] where -1 is interpreted as QUIT, [0,MAX_INT) is + interpreted as the amount of time to wait until continuing, and + MAX_INT is interpreted as WAIT. This could be more robust + because we'd be exposing the scheduling behavior of RA to iOS, + which might be good in other platforms as well. + + Approach 1 is the simplest and essentially just pushes down + these requirements to rarch_main_iterate. I have gone with the + "ugly way" first because it is the most expedient and + safe. Other eyeballs should decide if it isn't necessary. + */ + } + break; + case kCFRunLoopBeforeSources: + CF_LOG("RUNLOOP BEFORE SOURCES, frame: %d\n.", g_runloop.frames.video.count); + break; + case kCFRunLoopExit: + CF_LOG("RUNLOOP EXIT, frame: %d.\n", g_runloop.frames.video.count); + break; + case kCFRunLoopAllActivities: + CF_LOG("RUNLOOP ALL ACTIVITIES, frame: %d.\n", g_runloop.frames.video.count); + break; } - if (activity & kCFRunLoopBeforeTimers) - { - RARCH_LOG("RUNLOOP BEFORE TIMERS, frame: %d.\n", g_runloop.frames.video.count); - } - if (activity & kCFRunLoopAfterWaiting) - { - RARCH_LOG("RUNLOOP AFTER WAITING, frame: %d.\n", g_runloop.frames.video.count); - } - if (activity & kCFRunLoopBeforeSources) - { - RARCH_LOG("RUNLOOP BEFORE SOURCES, frame: %d\n.", g_runloop.frames.video.count); - } - if (activity & kCFRunLoopExit) - { - RARCH_LOG("RUNLOOP EXIT, frame: %d.\n", g_runloop.frames.video.count); - } - if (activity & kCFRunLoopAllActivities) - { - RARCH_LOG("RUNLOOP ALL ACTIVITIES, frame: %d.\n", g_runloop.frames.video.count); - } - if (activity & kCFRunLoopBeforeWaiting) -#endif - { -#ifdef DEBUG_RUNLOOP - RARCH_LOG("RUNLOOP BEFORE WAITING, frame: %d.\n", g_runloop.frames.video.count); -#endif - int ret = do_ra_iteration(); - if (ret == -1) - return; - CFRunLoopWakeUp(CFRunLoopGetMain()); - } - - - /* TODO/FIXME - I am almost positive that this is not necessary and is actually a - bad thing. - - 1st. Why it is bad thing. - - This wakes up the main event loop immediately and the main loop - has only one observer, which is this function. In other words, - this causes the function to be called immediately. I did an - experiment where I saved the time before calling this and then - reported the difference between it and the start of - do_iteration, and as expected it was about 0. As a result, when - we remove this, idle performance (i.e. displaying the RetroArch - menu) is 0% CPU as desired. - - 2nd. Why it is not necessary. - - The event loop will wake up itself when there is input to the - process. This includes touch events, keyboard, bluetooth, - etc. Thus, it will be woken up and without any intervention so - that we can process that event. - - Nota bene. Why this analysis might be wrong (and what to do about it). - - If RA is not idle and is running a core, then I believe it is - designed to expect to be called in a busy loop like this because - it implements its own frame timer to ensure that the emulation - simulation isn't too fast. In that case, this change would only - allow emulation to run when there was input, which would make - all games turn-based. :) - - There are two good ways to fix this and still have the desired - 0% CPU idle behavior. - - Approach 1: Change main_entry_decide from returning a boolean - (two-values) that are interpreted as CONTINUE and QUIT. Into - returning a char-sized enum with three values that are - interpreted as QUIT, WAIT, and AGAIN, such that QUIT calls - main_exit, WAIT doesn't wake up the loop, and AGAIN does. It - would then return AGAIN when a core was active. An ugly way to - get the same effect is to look have this code just look at - g_runloop.is_menu and use the WAIT behavior in that case. - - Approach 2: Instead of signalling outside of RA whether a core - is running, instead externalize the frame time that is inside - retroarch. change main_entry_decide to return a value in - [-1,MAX_INT] where -1 is interpreted as QUIT, [0,MAX_INT) is - interpreted as the amount of time to wait until continuing, and - MAX_INT is interpreted as WAIT. This could be more robust - because we'd be exposing the scheduling behavior of RA to iOS, - which might be good in other platforms as well. - - Approach 1 is the simplest and essentially just pushes down - these requirements to rarch_main_iterate. I have gone with the - "ugly way" first because it is the most expedient and - safe. Other eyeballs should decide if it isn't necessary. - */ } void apple_start_iteration(void)