Closed Bug 565323 Opened 10 years ago Closed 10 years ago

[OOPP] OSX: IPC severely degrades mouse input responsiveness

Categories

(Core :: Plug-ins, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: BenWa, Assigned: BenWa)

References

Details

(Keywords: perf)

Attachments

(1 file, 2 obsolete files)

Windows had a similar issue discussed in bug 558629. We will need to investigate if we need to throttle some messages. I'm going to start by investigating ConvertPoint and mouse events messages.
Keywords: perf
Attached patch Diagnostic Patch (obsolete) — Splinter Review
The following patch solves the problem. This patch only fires a single invalidate at a time and accumulates other invalidate rects while waiting for the next drawing operation. The patch is just a hack right now to prove that if the InvalidateRect events are properly handled then this problem goes away.

Without the patch it appears that the plug-in is requesting over 12 InvalidateRect before any redraw can happen. I think we need to deal better with situations where these events are being spammed. 

It might also be a good idea to additionally handle InvalidateRect being spammed within the PluginInstanceChild just to cut down on needless IPC messages.
Blocks: 567265
Marking as a beta1 blocker since this is a critical perf issue for Mac OOPP.
blocking2.0: --- → beta1+
InvalidateRect has a special code path on OS X that will accumulate invalidates if the code is currently drawing. The problem is that every time it accumulates it will queue a 'processPendingRedraws' message. 

With some inefficient Flash games each redraw will trigger several invalidates so we end up filling the queue with 'processPendingRedraws' messages when we only need one to process the accumulated regions. This would cause new invalidates to be processed right away by a 'processPendingRedraws' that was previously en-queued thus artificially raising redraw priority. Under certain race events this would cause the browser to spend more time redrawing then anything else and would cause the browser to 'hang' while the plug-in was still painting.

This patch will make sure we only have one 'processPendingRedraws' message in the queue at a time since it will clear all the accumulated invalidate rects.
Attachment #445118 - Attachment is obsolete: true
Attachment #447576 - Flags: review?(joshmoz)
Comment on attachment 447576 [details] [diff] [review]
Only queue one 'processPendingRedraws' message

Are you sure we don't need to init mPendingFullDisplay and mPendingDisplay member variables?
Attachment #447576 - Flags: review?(joshmoz) → review+
I fixed a style nit and initialized mPendingDisplay. mPendingFullDisplay has existed for awhile but if you want I can also initialize it.

I tried to find the rules on how fields are initialized in objective-c but couldn't find a good source. However other sections of this code seems to be relying on objective-c references being initialized to 'nil'. If this is not the case then this code might have bugs in it.
Attachment #447576 - Attachment is obsolete: true
Attachment #447633 - Flags: review?(joshmoz)
Attachment #447633 - Flags: review?(joshmoz) → review+
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/014ea2ea4998
Assignee: nobody → b56girard
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.