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

FIX count wall time instead of CPU time #9

Merged
merged 2 commits into from Mar 14, 2023

Conversation

wiiha
Copy link
Contributor

@wiiha wiiha commented Mar 9, 2023

Intervall only accounts for CPU time
passed, not actual wall time passed
resulting in the timer going slower
than actual wall time.

CPU time vs Wall time
When a browser tab or window is off-screen
or in a tab that isn't focused, the scheduler
doesn't execute the JS engine every
tick/second. Instead, the process is scheduled
to run in less frequent intervals. This means
that one can't assume that the interval timer
will execute once every wall time second, but
rather once every "JS engine active" second.
Since the countdown should count wall time,
we need to calculate a timestamp based delta
for when the function last ran.

Intervall only accounts for CPU time
passed, not actual wall time passed.

CPU time vs Wall time
When a browser tab or window is off-screen
or in a tab that isn't focused, the scheduler
doesn't execute the JS engine every
tick/second. Instead, the process is scheduled
to run in less frequent intervals. This means
that one can't assume that the interval timer
will execute once every wall time second, but
rather once every "JS engine active" second.
Since the countdown should count wall time,
we need to calculate a delta for when the
function last ran.
@vercel
Copy link

vercel bot commented Mar 9, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @agustinl on Vercel.

@agustinl first needs to authorize it.

@wiiha
Copy link
Contributor Author

wiiha commented Mar 9, 2023

@agustinl, if you would like me to open the PR against another branch feel free to write that in a comment. 👍
I have used this almost daily since you presented it on HN. It's a very nice experience.

@agustinl agustinl added enhancement New feature or request good first issue Good for newcomers labels Mar 10, 2023
@agustinl
Copy link
Owner

agustinl commented Mar 10, 2023

Hi @wiiha ! Thanks for your time to this PR!

I found an issue: when you pause the pomodoro timer, its still running in background, so when you play again, they discount the seconds that it was paused.

On the other hand, do you have any example of how to replicate this issue?

Time passing while paused was accounted for
when the timer was again started. It should
not be the case.
@wiiha
Copy link
Contributor Author

wiiha commented Mar 11, 2023

Nice catch @agustinl, I fixed the pause now.
Regarding replicating the issue. Execute the following code snippet in the dev tools console and then move to another tab (or let the tab be off screen) for say 5 minutes. When you come back you should see a difference in the time output.

var t0 = Date.now();
var intervallSeconds = 1;
setInterval(()=>{
   console.log({intervallSeconds,timestampSeconds:Math.round((Date.now()-t0)/1000)})
    intervallSeconds = intervallSeconds + 1
},1000)

Before switching tab
image

After switching tab and coming back
image

@agustinl
Copy link
Owner

Works perfectly @wiiha 🙌

Merge to release v4.0.0

Thanks for your support!

@agustinl agustinl changed the base branch from main to v4.0.0 March 14, 2023 02:34
@agustinl agustinl merged commit be2389c into agustinl:v4.0.0 Mar 14, 2023
@agustinl agustinl mentioned this pull request Mar 14, 2023
@wiiha wiiha deleted the fix/timers-follow-wall-time branch March 16, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants