Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX/ENH: macos: dispatch timer tasks asynchronously to the main loop#29030
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Previously, the timers were dependent on the length of time it took forthe timer callback to execute. This dispatches the callback tothe task queue to avoid synchronously waiting on long-running callbacktasks.
| // we shouldn't do it ourselves when the object is deleted. | ||
| self->timer = NULL; | ||
| } | ||
| self->timer = [NSTimer scheduledTimerWithTimeInterval: interval |
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.
This automatically schedules the timer on the main runloop for us.
https://developer.apple.com/documentation/foundation/timer/2091889-scheduledtimer
| self->timer = [NSTimer scheduledTimerWithTimeInterval: interval | ||
| repeats: !single | ||
| block: ^(NSTimer *timer) { | ||
| dispatch_async(dispatch_get_main_queue(), ^{ |
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.
This is the major change here. We are dispatching our event asynchronously (so as to not sequentially depend on the callback processing time), but putting it onto the main thread's queue.
closing in favor of doing this all in a consolidated PR:#29062 |
PR summary
Previously, the timers were dependent on the length of time it took for the timer callback to execute. This dispatches the callback to the main thread's task queue to avoid synchronously waiting on long-running callback tasks.
Note that I didn't add any tests here because there is inconsistency between backends, so other backends fail for these assertions currently. I've opened up an issue to discuss that:#29029
On main, this script prints every 2 seconds
callback time + timer interval, with this PR it prints every secondmax(callback time, timer interval).PR checklist