Bug 99939 - should disallow gc/layout/stylerecalc in beforeunload/unload handlers
Summary: should disallow gc/layout/stylerecalc in beforeunload/unload handlers
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-21 10:26 PDT by Ojan Vafai
Modified: 2017-02-19 21:04 PST (History)
11 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 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).
Comment 1 Alexey Proskuryakov 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?
Comment 2 James Robinson 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.
Comment 3 Ojan Vafai 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.
Comment 4 Ojan Vafai 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.
Comment 5 James Robinson 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.
Comment 6 Ryosuke Niwa 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...