SetFrameRate thread wait CPU issue

[danoli3]

i confirm similar behaviour (macOS 14, M2Pro 16'', built-in display):

no ofSetFrameRate: 8-10% CPU for ~100fps, windowed or game mode.
ofSetFrameRate(60): 90-100% CPU for 60fps, windowed, or game mode, or fullscreen with game mode disabled
so game mode itself seems unrelated to this behaviour.

digging a bit, there is a new class ofTimerFps, which contains a sort of yielding loop that is consuming cycles -- commenting out these lines:

https://github.com/openframeworks/openFrameworks/blame/1c2a586473e5c2c4e5eefac3f3898f6c9e8c3b70/libs/openFrameworks/utils/ofTimerFps.cpp#L30-L32

stops the CPU overload issue. doing std::this_thread::sleep_until(wakeTime - 36ms); seems odd as it would never sleep unless a very slow (27.7fps?) frame rate? the PR is #7867 and quickly scanning it gives the information that intensively spinning yields diminishes jitter, but maybe CPU usage was not considered?

maybe this should be considered a new time mode? ofSetTimeModeFixedAndAmped()? or something like that... in any case allowing the previous mode is always good as new behaviour may interact in unexpected ways for existing projects.

[dimitre]

We can solve the CPU usage (measure) issue by just changing

	   std::this_thread::yield();

to

	   std::this_thread::sleep_for(5us);

It will fix the CPU usage measuring (more on that later if needed)
but if we want to see a broader picture we can inspect energy tab.
using yield it is more stable, using sleep_for is fluctuates a lot more.

we can decrease a bit the 36ms also, but it increases FPS jitter
so if we want to make let's say a MIDI sequencer, it is best to "wake early"

cc @danomatika @artificiel @ofTheo

[artificiel]

I'd reiterate the principle of implementing this as a new time mode instead of replacing an existing one — enabling the user to determine if they want a fixed frame with no CPU cycles (previous behaviour), or fixed frame rate with tighter timing but higher CPU cycles by calling ofSetTimeModeFixedAndAggressive() (not sure about the name, but it should retain the Fixed aspect, plus a qualificative that denotes what's going on within it).

that way we can experiment with this new approach without forcing all the OF projects that call ofSetFrameRate() to suddenly behave differently.

[danomatika]

My thought is more to leave it as it was but provide either an option to turn on high frequency timing and/or provide a callback or way for users to implement their own idle function. AN associated example might be helpful too.

I feel like this is one of those things that can be highly dependent upon the project. It reminds me of audio projects where some users are fine with 16ms+ latency while a few require 2ms always however that is going to burn CPU so most programs do not enable by default.

EDIT: Also, please please please document the pros and cons of this. We want to avoid the inevitable issue where someone complains that they turned on the cool sounding option but now their app now burns through battery and they don't know why when it didn't before... but it sure is smooth. 😄

I think the naming should be carefully chosen if it is an *enable function.

[dimitre]

Great @danomatika let us know which is the ideal place to discuss other than the discussion already existent in the PR.

Ideas:
if we lower the waketime for something like 8ms and change yield to std::this_thread::sleep_for(5us);
it will have CPU usage similar to 0.12.0
but we still benefit of straight chrono (using ratios to keep the best precision possible) and being high level, it takes care of any differences in platforms.

But it is only my opinion and I'm good if anybody wants it reverted

[ofTheo]

I like your suggestion @dimitre

if we lower the waketime for something like 8ms and change yield to std::this_thread::sleep_for(5us);
it will have CPU usage similar to 0.12.0

and the option to switch to more precise if needed.

[dimitre]

Ok the option for more precision is not needed for 0.12.1.

[danomatika]

I'd prefer if there were three frame rate precision/timing options: none (as before), low, and high.

I had no issues with how things were before and frame rate consistency has really never been an issue for my projects.

[danomatika]

Also, on some projects which poll sensors or events in a loop, I provide an idle/sleep time option on the command line to fine tune it by setting you own value in ms/us etc. Maybe this would work and the presents would be number defines with none being 0, a special case to turn it off.

UPDATE: Maybe something like:

#define OF_FRAMERATE_SLEEP_NONE 0
#define OF_FRAMERATE_SLEEP_LOW 8
#define OF_FRAMERATE_SLEEP_HIGH 32

// set the frame rate idea sleep timing in us,
// lower values improve frame rate jitter (aka smoother) but use higher CPU 
ofSetFrameRateSleep(unsigned int t);

I haven't looked at the actual values, so 8 and 32 are guesses.

[danomatika]

Also, to explain my reasoning to keep "none" as default for now:

1. This is a big fix release. I wouldn't change something internal like this and potentially surprise people with new behavior. Provide it and document it with a note in the release info, then people can do more experimentation to sus out any other issues. Make the default setting change in OF 0.13.
2. As stated, I haven't any issue with the previous behavior personally.

I am not against this on a whole, just the move toward making it default right now.

[artificiel]

actually 3 things are happening within the PR:

• a) change the System time mode timing mechanism from platform-specific nanosleep to generic thread::wait_until
• b) implement the audacious loop of intensely yielding for the effect of less jitter
• c) adapt the time units to std::chrono

as a whole, this is arguably are an enhancement, but (a) and (b) have enough effect to throw off existing projects/expectations (as per @danomatika's reaction) and additionally are not fixing a well-defined problem. as such it is evident that the change is controversial and should not be introduced as a default, even if the CPU usage symptom is fixed, as the ramifications of the change may be deeper than apparent.

given that OF already supports System and Filtered and FixedRate as time modes, I would recommend wrapping this new functionality in a new timeMode, named something like HighPrecision, which could be activated with arguments to fine-tune the algorithm ofSetTimeModeHighPrecision(waketime=8ms, sleepfor=5us). this would enrich the API without imposing changes on existing projects that already work fine.

NB there is also room for API improvement, as the "timeMode" concept is used in many classes, sometimes differently such as when setting to Filtered, the Events are Filtered, but the actual clock stays System; this suggest the different timeMode-related methods and enums should be more precisely named to diminish ambiguity and facilitate understanding of the relationships between these.

openFrameworks/libs/openFrameworks/utils/ofUtils.cpp

Lines 302 to 303 in 3dff4e2

	window->events().setTimeModeFiltered(alpha);
	of::priv::getClock().setTimeModeSystem();

[ofTheo]

I believe this was fixed in #8406
@dimitre feel free to re-open if I am incorrect.

[danomatika]

I think so too. The of_v20250330_osx_release build is working for me as before.