Closed Bug 27380 Opened 25 years ago Closed 25 years ago

Too many XQueryPointer() calls

Categories

(Core :: XUL, defect, P3)

x86
Linux
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: blizzard, Assigned: blizzard)

Details

jlnance is the actual reporter here. I'm just opening this bug to track this performance issue. Hello All, I was supprised that mozilla called select() since the gtk event loop is based on poll(). I decided to go through the code with gdb and see where it was getting called (its not a necessarily a problem, I just wanted to know). As a result of this, I noticed that we call XQueryPointer() A LOT, and from what I can tell from gdb, each call requires a round trip to the X server. I think some of these calls may be unnecessary, and if they could be removed it would probably help our speed, particulary when running over slow links. The first thing I noticed is that there is some debugging code that checks to see if the caps lock key is depressed, and it dumps out some info if it is. Fortunatly this code is only included for debug builds. It calls XQueryPointer() via gdk_keyboard_get_modifiers() quite often. Take a look in widget/src/gtk/nsWidget.cpp and search for CAPS_LOCK_IS_ON. Even though its debug only code, it would be nice if it did not have to talk to the X server so often. Could this be improved? After I got rid of the CAPS_LOCK_IS_ON stuff, I found that we were still calling it a lot from here: #0 0x40734c17 in nsWidget::OnMotionNotifySignal (this=0x85d3dd8, aGdkMotionEvent=0x8170f30) at nsWidget.cpp:1614 1609 if (aGdkMotionEvent) 1610 { 1611 x = (gint) aGdkMotionEvent->x; 1612 y = (gint) aGdkMotionEvent->y; 1613 1614 > gdk_window_get_pointer(aGdkMotionEvent->window, &x, &y, nsnull); 1615 1616 event.point.x = nscoord(x); 1617 event.point.y = nscoord(y); 1618 event.widget = this; This confused me because it looks like the code is saying "the mouse moved to position (x,y), go find out where the mouse is", which of course would be redundant. I tried commenting out line 1614, but I found that it is needed. Still it seems like that if we are recieving and processing MotionNotify events, we should be able to figure out where the mouse is just by looking at the events. Even with 1641 commented out, we still make a lot of calls to XQueryPointer() through what appears to be a very similar stack trace (sorry I did not save it). This makes me wonder if 2 objects both need the pointer position when the pointer moves, and they each call XQueryPointer() themselves to get it. If this is true, then perhaps this could be folded into 1 call.
Fix checked in.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
please ignore, massive spam giving jrgm@netscape.com backlog of XPToolkits resolved fixed bugs to verify
QA Contact: paulmac → jrgm
Please ignore the spam. Changing address.
Assignee: blizzard → blizzard
Status: RESOLVED → NEW
remarking as FIXED
Status: NEW → RESOLVED
Closed: 25 years ago25 years ago
jim_nance: You said "I tried commenting out line 1614, but I found that it is needed.", but blizzard's checkin 02/15 removes this line. Is this correct? [Feel free to ignore me :-] I'm not supposed to be looking at the code].
I seem to remember fixing what it was depending on so I could remove the actual call.
Sounds good to me (I assumed this was the case, but was having a little logic disconnect between jim's note and the outcome). Marking le verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.