RESOLVED WONTFIX 99939
should disallow gc/layout/stylerecalc in beforeunload/unload handlers
https://bugs.webkit.org/show_bug.cgi?id=99939
Summary should disallow gc/layout/stylerecalc in beforeunload/unload handlers
Ojan Vafai
Reported 2012-10-21 10:26:11 PDT
We should make beforeunload/unload handlers as cheap as possible as they're on the critical path for closing tabs/windows and navigating. I can't think of any use-cases where we'd want to do a gc, layout or style recalc in a beforeunload/unload handler. Other than adding the hooks to disable these things, we'd also need to have hooks to reenable them if the user stays on the page after the beforeunload event. We might want to do the same for pagehide events? Not sure how feasible it is to disable these things. Disabling gc seems like it shouldn't be too hard. layout/style-recalc might be harder since they don't always start at the root node. Are there other things we could disable during these events? Maybe it's time that we kill sync XHR from these events (e.g. just treat them as async).
Attachments
Alexey Proskuryakov
Comment 1 2012-10-22 15:21:32 PDT
Do users actually complain about page closing being slow? What is the problem to solve by making these faster?
James Robinson
Comment 2 2012-10-22 15:41:27 PDT
What would you return to layout-dependent JS queries in beforeunload/unload handlers? I doubt that disabling GC would really make things better, unless the whole process was going to be killed just after the (before)unload handler finished. Otherwise it's just moving cost around that would happen soon anyway.
Ojan Vafai
Comment 3 2012-10-22 15:55:22 PDT
(In reply to comment #1) > Do users actually complain about page closing being slow? Yes. Also, we have data that ~5% of unload handlers take >449ms to run. ~2% hit the 1 second time limit that chromium enforces when it can (e.g. when closing the tab or when doing a cross-process navigation). Of the ones that don't get killed, some take longer than 10 seconds to run. It's not clear to me that the cost here is at all related to gc/layout/stylerecalc though. The first step towards addressing this bug would be to add measurements so we can see if there would be any real benefit from doing this.
Ojan Vafai
Comment 4 2012-10-22 15:56:35 PDT
(In reply to comment #2) > What would you return to layout-dependent JS queries in beforeunload/unload handlers? Stale numbers? > I doubt that disabling GC would really make things better, unless the whole process was going to be killed just after the (before)unload handler finished. Otherwise it's just moving cost around that would happen soon anyway. Yes. This would potentially help for tab closing and cross-process navigations. Again, we should histogram this stuff first.
James Robinson
Comment 5 2012-10-22 18:46:56 PDT
(In reply to comment #4) > (In reply to comment #2) > > What would you return to layout-dependent JS queries in beforeunload/unload handlers? > > Stale numbers? This sounds like a complete non-starter to me. The page might not actually unload when these events are fired and there's tons of layout-dependent JS out there for seemingly trivial things. Silently returning incorrect values is likely to break pages in ways that will be nearly impossible to debug. I agree that measuring the breakdown of costs as the logical next step.
Ryosuke Niwa
Comment 6 2017-02-19 21:04:30 PST
I don't think we're going to do this. There are better ways to mitigate the cost of beforeunload, etc...
Note You need to log in before you can comment on or make changes to this bug.