Skip to content

Conversation

@iangrunert
Copy link
Contributor

@iangrunert iangrunert commented Mar 24, 2025

70d5b92

[Win] Improvements to run loop timer resolution
https://bugs.webkit.org/show_bug.cgi?id=284823

Reviewed by Yusuke Suzuki.

WM_TIMER has a 10ms resolution, which means we can't hit 60fps on
requestAnimationFrame. By using MsgWaitForMultipleObjectsEx and
maintaining our own timer queue we can get better timer resolution.

* Source/WTF/wtf/RunLoop.h:
* Source/WTF/wtf/win/RunLoopWin.cpp:
(WTF::RunLoop::wndProc):
(WTF::RunLoop::run):
(WTF::RunLoop::msTillNextTimer):
(WTF::RunLoop::fireTimers):
(WTF::RunLoop::cycle):
(WTF::RunLoop::TimerBase::timerFired):
(WTF::RunLoop::TimerBase::start):
(WTF::RunLoop::TimerBase::stop):

Canonical link: https://commits.webkit.org/292846@main

ea2075e

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision 🧪 mac-AS-debug-wk2 ⏳ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@iangrunert iangrunert self-assigned this Mar 24, 2025
@iangrunert iangrunert force-pushed the ig/bug-284823-win-run-loop-improvements branch from 79805fc to b8a3e2e Compare March 24, 2025 16:49
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 24, 2025
@iangrunert iangrunert force-pushed the ig/bug-284823-win-run-loop-improvements branch from b8a3e2e to b9d5665 Compare March 24, 2025 20:59
@iangrunert iangrunert force-pushed the ig/bug-284823-win-run-loop-improvements branch from b9d5665 to 9b6d578 Compare March 27, 2025 21:12
@iangrunert iangrunert force-pushed the ig/bug-284823-win-run-loop-improvements branch from 9b6d578 to ea2075e Compare March 27, 2025 21:22
@iangrunert iangrunert added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Mar 28, 2025
https://bugs.webkit.org/show_bug.cgi?id=284823

Reviewed by Yusuke Suzuki.

WM_TIMER has a 10ms resolution, which means we can't hit 60fps on
requestAnimationFrame. By using MsgWaitForMultipleObjectsEx and
maintaining our own timer queue we can get better timer resolution.

* Source/WTF/wtf/RunLoop.h:
* Source/WTF/wtf/win/RunLoopWin.cpp:
(WTF::RunLoop::wndProc):
(WTF::RunLoop::run):
(WTF::RunLoop::msTillNextTimer):
(WTF::RunLoop::fireTimers):
(WTF::RunLoop::cycle):
(WTF::RunLoop::TimerBase::timerFired):
(WTF::RunLoop::TimerBase::start):
(WTF::RunLoop::TimerBase::stop):

Canonical link: https://commits.webkit.org/292846@main
@webkit-commit-queue webkit-commit-queue force-pushed the ig/bug-284823-win-run-loop-improvements branch from ea2075e to 70d5b92 Compare March 28, 2025 19:51
@webkit-commit-queue
Copy link
Collaborator

Committed 292846@main (70d5b92): https://commits.webkit.org/292846@main

Reviewed commits have been landed. Closing PR #42936 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 70d5b92 into WebKit:main Mar 28, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Mar 28, 2025
@fujii
Copy link
Contributor

fujii commented Mar 28, 2025

Unfortenately, this change made layout tests crashy. Even though EWS actually catched a crash, but not reported it with a red bubble. I'm going to revert this change temporarily. Will take a look next week.

if (m_liveTimers.contains(wParam))
timer = std::bit_cast<RunLoop::TimerBase*>(wParam);
}
timer = std::bit_cast<RunLoop::TimerBase*>(wParam);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timer can be already destroyed here. I fixed it in #43244

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.

6 participants