It is much faster to reschedule with CFRunLoopTimerSetNextFireDate than to repeatedly delete and create timers.
Created attachment 188213 [details] patch
Comment on attachment 188213 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=188213&action=review Hilarious! r+ from me since this looks like correct use of the API to me, though you might wanna double-check with someone with more CF knowledge. > Source/WebCore/platform/mac/SharedTimerMac.mm:154 > + static CFRunLoopTimerRef timer; > + if (!timer) { You could use dispatch_once() for this block.
http://trac.webkit.org/changeset/142825
When I first implemented SharedTimer I tried using CFRunLoopTimerSetNextFireDate and it didn’t work. Maybe I did it wrong. Or maybe there were CF bugs that have since been fixed. Someone should be sure to test this on the oldest OS X and iOS versions that WebKit currently supports in case that old bug is not fixed there.
(In reply to comment #4) > When I first implemented SharedTimer I tried using CFRunLoopTimerSetNextFireDate and it didn’t work. Maybe I did it wrong. Or maybe there were CF bugs that have since been fixed. Someone should be sure to test this on the oldest OS X and iOS versions that WebKit currently supports in case that old bug is not fixed there. JSC already uses CFRunLoopTimerSetNextFireDate for HeapTimer.
Re-opened since this is blocked by bug 109856
This caused failures in some inspector debugger tests http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r142826%20(5930)/results.html:
Maybe nested event loops are causing problems or something?
Created attachment 188716 [details] patch2 try to fix the inspector debugger
Comment on attachment 188716 [details] patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=188716&action=review > Source/WebCore/platform/mac/SharedTimerMac.mm:177 > +void reinsertSharedTimer() > +{ > + // For some reason the timer won't fire in a nested runloop unless it has been freshly inserted. > + CFRunLoopTimerInvalidate(globalSharedTimer); > + CFRelease(globalSharedTimer); > + globalSharedTimer = 0; > +} This function's name seems to be at odds with its contents. Where does the timer actually get re-inserted?
(In reply to comment #10) > This function's name seems to be at odds with its contents. > Where does the timer actually get re-inserted? From sharedTimer(), next time the timer is accessed. This will happen from ThreadTimers::fireTimersInNestedEventLoop() right after this call. Suggestions welcome.
Comment on attachment 188716 [details] patch2 View in context: https://bugs.webkit.org/attachment.cgi?id=188716&action=review r=me if you find a nicer color for the bikeshed. > Source/WebCore/platform/mac/SharedTimerMac.mm:171 > +void reinsertSharedTimer() I'd just prefer to call this removeFoo() instead of reinsertBar() since that's what it does.
http://trac.webkit.org/changeset/143210
Re-opened since this is blocked by bug 110128
The bots still see some nested runloop related failures.
Comment on attachment 188213 [details] patch Cleared Andreas Kling's review+ from obsolete attachment 188213 [details] so that this bug does not appear in http://webkit.org/pending-commit.