Skip to content

fix: tick timer instead of using Date.now#3494

Closed
ronag wants to merge 1 commit intomainfrom
timer-tick
Closed

fix: tick timer instead of using Date.now#3494
ronag wants to merge 1 commit intomainfrom
timer-tick

Conversation

@ronag
Copy link
Member

@ronag ronag commented Aug 23, 2024

Partially fixes some of the timeout issues we have been facing. Again needs work with tests.

Refs: #3493

This relates to...

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@mcollina
Copy link
Member

I'm generically ok with the change, but I would be happier with tests. Maybe one from @Uzlopak's PR?

@ronag
Copy link
Member Author

ronag commented Aug 25, 2024

I suspect that pr includes this change somewhere

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 25, 2024

This PR is replacing the Date.now() call with saying that every time setTimeout is triggered that 499 ms are passed. This means, that if the event loop blocks for e.g. 900ms, we still "think" that just 499ms passed. So the fasttimer will be artificially delayed.

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 25, 2024

We could change it in my PR to just increment the fastNow value instead of calling performance.now. But somehow it feels wrong.

@ronag
Copy link
Member Author

ronag commented Aug 25, 2024

The whole point is that we don't want to include event loop delay

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 25, 2024

adapted with unit test
ad339a2

@ronag ronag closed this Aug 25, 2024
@Uzlopak Uzlopak deleted the timer-tick branch September 6, 2024 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants