Kevin diagnosed and found this issue. I'm just filing for him. In bug 579488 (typing in twitter) nsWindow::DispatchStarvedPaints forces us to paint after every character typed. The paints take a long time. This forces us to paint more often, instead of coalescing paints, making things worse then they could be. We should be smarter about dispatching starved paints. There are some thoughts about this (for Linux) in bug 602303.
Created attachment 505712 [details] [diff] [review] Throttle dispatching of starved paints so that time is allowed for processing input events I tried throttling the starved paint logic so that it will only dispatch a starved paint synchronously if it's been at least 50 milliseconds since the last paint completed. This drastically improves the responsiveness of the twitter input field for me without compromising the responsiveness of the sinatra network graph on GitHub (from one of the previously mentioned bug). This seems like a decent way to solve the problem.
I like this! + // The point in time at which the last paint completed. We use this to avoid + // painting too rapidly in response to frequent input events. + mozilla::TimeStamp mLastPaintEndTime; add "typedef mozilla::TimeStamp TimeStamp;" to the top of nsWindow to avoid mozilla:: prefixes.
I'm not sure if this is too risky or not for FF4, but if we want it for FF4 we should get it landed soon so it can get testing.
I'd approve a final patch.
Created attachment 506605 [details] [diff] [review] Throttle dispatching of starved paints so that time is allowed for processing input events (2) Patch revised based on feedback and tested locally. Spent a bit trying to come up with a good automated test but could not come up with one that would perform well without a lot of effort. Andreas suggested that a manual test might be sufficient here. My manual test case for this is, roughly: 1. Load up new twitter and expand the tweet input field so it is around 5 lines tall and horizontally covers the right pane of their UI. This makes paints expensive enough to reproduce the issue. 2. Rapidly spam keystrokes into the input field (holding down a key is not sufficient) for around 15 seconds, then stop. Without the patch, you should observe Firefox slowly processing each keystroke for multiple seconds after you stop typing, until it eventually becomes responsive again. With the patch, it should become responsive within a second of your last keystroke.
--- a/widget/src/windows/nsWindow.h Fri Jan 21 04:25:18 2011 +0100 +++ b/widget/src/windows/nsWindow.h Mon Jan 24 17:54:29 2011 -0800 @@ -60,7 +60,12 @@ #include "gfxWindowsSurface.h" #include "nsWindowDbg.h" #include "cairo.h" -#include "nsITimer.h" +#include "nsITimer.h" +#include "mozilla/TimeStamp.h" + +typedef mozilla::TimeStamp TimeStamp; +typedef mozilla::TimeDuration TimeDuration; Sorry, we shouldn't pollute the global namespace like this. Put these typedefs inside the nsWindow class.
Created attachment 506626 [details] [diff] [review] Throttle dispatching of starved paints so that time is allowed for processing input events (3) Ah, my mistake. Moved the typedefs into the correct place.
I had to make one small change because we can call DispatchPendingEvents before we've had a paint, but otherwise green on try server. I'll push this later today.
Backed this out to fix bug 635465 http://hg.mozilla.org/mozilla-central/rev/88afdccd3ba6
So in order to fix this again we need to fix bug 635465 in a better way, perhaps bug 635465, comment 12.
Comment on attachment 506626 [details] [diff] [review] Throttle dispatching of starved paints so that time is allowed for processing input events (3) (removing the patch approval for bookeeping)
I'm not comfortable landing this on cedar...
This shouldn't land. It was backed out for causing a regression and we haven't figured out how to fix the regression yet.