Skip to content
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

Timer running slow #32

Closed
klamping opened this issue Nov 27, 2018 · 12 comments
Closed

Timer running slow #32

klamping opened this issue Nov 27, 2018 · 12 comments
Labels
bug Something isn't working

Comments

@klamping
Copy link

I'm on Mac OSX (HIgh Sierra) and I have noticed the timer is running slow. I've compared this to a manual stopwatch and it lags about ~10 by the end of the 25 minute session (so it's reporting I still have 10 minutes left when a full 25 minutes have passed).

No idea what's causing this. I usually start the timer, then minimize it while I work away. Haven't tried running it solo with nothing else running.

@Splode Splode added the bug Something isn't working label Nov 28, 2018
@NeonArray
Copy link

NeonArray commented Dec 10, 2018

I just wanted to chime in cause this caught my interest. I haven't tried replicating this issue, but I had taken a quick look and noticed you are using setInterval to handle the timer. This could be the issue as setInterval isn't really dependable to fire consistently and accurately.

I found an npm module called tocktimer that you could look into replacing the setInterval.

I noticed also there is also something else you may try (or as well as using this with the npm package) that I found in the Electron BrowserWindow API. The backgroundThrottling boolean value can be set to false.

backgroundThrottling Boolean (optional) - Whether to throttle animations and timers when the page becomes background. This also affects the Page Visibility API. Defaults to true.

@Splode
Copy link
Owner

Splode commented May 19, 2019

The lag that you're describing, @klamping, may be fixed with the v0.6.1 release. @winktim refactored the timer to use setTimeout instead of setInterval with #41.

@ocularrhythm, I'll look into the backgroundThrottling option to see if it offers improvements. Thanks for the suggestion!

@bencfd
Copy link

bencfd commented Jun 5, 2019

I installed v0.6.1 and I notice a 10 minute lag by the third work session. Could it be related to privacy settings / access privileges? I'm running macOS 10.14.5 (Mojave). I'd be happy to provide more details if I can, hoping it helps.

@jellium
Copy link

jellium commented Jun 5, 2019

I believe I have the same issue on Windows 7 with latest release (0.6.1). It appears that the timer runs two times slower than actual time when minimized.

@jonathanTIE
Copy link

I'm confirming the same problem of time going about 2 time slower when minimized with latest release on Windows 10.

@jonathanTIE
Copy link

jonathanTIE commented Jun 12, 2019

I've done some testing for this issue
With "0.6.2"(dev branch ->Set interval and backgroundthrottle set to false), got about 40s lag in only 25min.
With "0.6.1" modified,(setTimeout, and added backgroundthrottle set to false), I got 10s lag in 25 min.
I woud stick with the current fix in 0.6.1 and just adding the backgroundthrottle before we can come up with a better solution.

@twinkelmann
Copy link
Contributor

To improve accuracy, we could store both the start time and end time of the timer, and when displayed, update the current second based on theses (current - start)

That way, even if the app is throttled in the background, we can still check pretty much every second if the timer should have ended (current >= end), thus the delay should only be at most the throttling time.

I can propose a PR of that implementation if you think that's a good idea.

@Splode
Copy link
Owner

Splode commented Jun 13, 2019

@jonathanTIE I'm seeing similar results between the 2 versions. It seems that in either case the timer is most inconsistent when the application is minimized. I think for now, let's go with your recommendation and stick with 0.6.1. Thanks for running those tests!

@jonathanTIE
Copy link

jonathanTIE commented Jun 13, 2019

I've continued to do some testing, the problem is the "computedtimeout" that set the timeout.
I don't know why but the time between the 2 calls of getMilliseconds() (at the beggining and end of the timer loop) seems so high that it passes on the next second.
Like for example, when the application is minimized, instead of being à T=0.931s then at the end of the loop at T=0.938s, we end up from T=0.931s to T=1.424s in real time which translate in T=0.424s.
so new Date().getMilliseconds() - msOffset is a negative number, which make computedTimeout higher than 1000ms, and it now hold the value needed for waiting for 2s instead of 1.
This solution seems to work, still need to check if lags could happen but it prevent the "2 times slower" problem by calculating the right "lag" that happened between the beggining and end of TimerLoop.

Just change computedTimeout :
const computedTimeout = 1000 - Math.abs((new Date().getMilliseconds() - msOffset))

jonathanTIE added a commit to jonathanTIE/pomotroid that referenced this issue Jun 13, 2019
@twinkelmann
Copy link
Contributor

I've continued to do some testing, the problem is the "computedtimeout" that set the timeout.
I don't know why but the time between the 2 calls of getMilliseconds() (at the beggining and end of the timer loop) seems so high that it passes on the next second.
Like for example, when the application is minimized, instead of being à T=0.931s then at the end of the loop at T=0.938s, we end up from T=0.931s to T=1.424s in real time which translate in T=0.424s.
so new Date().getMilliseconds() - msOffset is a negative number, which make computedTimeout higher than 1000ms, and it now hold the value needed for waiting for 2s instead of 1.
This solution seems to work, still need to check if lags could happen but it prevent the "2 times slower" problem by calculating the right "lag" that happened between the beggining and end of TimerLoop.

Just change computedTimeout :
const computedTimeout = 1000 - Math.abs((new Date().getMilliseconds() - msOffset))

That won't really fix it. Please see my PR (coming in minutes).
I re-implemented the timer using requestAnimationFrame.

@olavfosse
Copy link

It is still slow.

@Splode Splode mentioned this issue Jan 5, 2020
@Splode
Copy link
Owner

Splode commented Jan 6, 2020

I've moved the timer functionality to a web worker with #68. This allows the timer to run off the main render thread, which should decrease the SetInterval throttling, especially when running in the background.

In initial tests on Windows 10-1909 and Ubuntu 19.10, the timer accuracy of v0.7.0 is significantly improved over v0.6.2. It might require more testing on various systems, but in any case, this looks to be an overall improvement.

The only drawback that I can see to this implementation is increased complexity.

I'll leave this issue open for a few weeks to facilitate feedback and discussion.

@Splode Splode closed this as completed Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants