REOPENED 134109
Increase priority on SharedTimer source
https://bugs.webkit.org/show_bug.cgi?id=134109
Summary Increase priority on SharedTimer source
Andre Moreira Magalhaes
Reported 2014-06-20 10:34:16 PDT
While running webkit on a Debian virtual machine with limited resources I stumbled upon an issue where JS setTimeout callbacks were not being properly invoked due to machine limitations and the usage of a low priority on setTimeout timers. After discussing the issue with Gustavo (kov), we decided to increase the SharedTimer priority to use the default glib priority. I would like to start a discussion on whether this is the proper way to fix this issue. Patch to come.
Attachments
Patch (1.63 KB, patch)
2014-06-20 10:38 PDT, Andre Moreira Magalhaes
no flags
Patch with proper changelog (2.73 KB, patch)
2014-06-20 10:42 PDT, Andre Moreira Magalhaes
no flags
Andre Moreira Magalhaes
Comment 1 2014-06-20 10:38:41 PDT
Andre Moreira Magalhaes
Comment 2 2014-06-20 10:42:41 PDT
Created attachment 233434 [details] Patch with proper changelog Oops, forgot to update changelog, new patch in.
Gustavo Noronha (kov)
Comment 3 2014-06-27 08:19:13 PDT
Comment on attachment 233434 [details] Patch with proper changelog This would be a problem for webkit1, but is a proper fix for the webkit2 model IMO, since we do not compete with GTK+ mainloop sources anymore.
WebKit Commit Bot
Comment 4 2014-06-27 08:51:28 PDT
Comment on attachment 233434 [details] Patch with proper changelog Clearing flags on attachment: 233434 Committed r170529: <http://trac.webkit.org/changeset/170529>
WebKit Commit Bot
Comment 5 2014-06-27 08:51:30 PDT
All reviewed patches have been landed. Closing bug.
Carlos Alberto Lopez Perez
Comment 6 2014-07-16 07:23:26 PDT
This caused a performance regression on the test Animation/balls:FrameRate. See: https://bugs.webkit.org/show_bug.cgi?id=134972
Martin Robinson
Comment 7 2014-07-16 08:01:31 PDT
The priority of the timers is very sensitive and can negatively affect animations, so I recommend we roll this out until we understand the isseu.
Andre Moreira Magalhaes
Comment 8 2014-07-16 08:43:16 PDT
(In reply to comment #7) > The priority of the timers is very sensitive and can negatively affect animations, so I recommend we roll this out until we understand the isseu. I agree here. I actually opened this bug more to start a discussion on the proper priorities to use for the various timers/sources as we had some issues with JS being starved and thus breaking some webapps that relied on setTimeout calls. This issue only happened in a very specific usecase where we had a browser running in a VM with limited resources. I vote for reverting this but would like to see your opinion on what is the best approach to solve the issue with JS being starved when running on limited resources.
Carlos Alberto Lopez Perez
Comment 9 2014-07-22 08:07:52 PDT
I reverted r170529 on r171343 <https://trac.webkit.org/r171343> because it caused a ~10% performance regression on the perf test Animation/balls: https://webkit.org/b/134972 Reopening this bug.
Note You need to log in before you can comment on or make changes to this bug.