347952 - X server roundtrips positioning invisible popups can hurt performance

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In some work I did a couple of years ago, I made popups be toplevel widgets and added code to position them here:
http://lxr.mozilla.org/mozilla/source/view/src/nsView.cpp#374
This requires an X server roundtrip in general. This can get severe when we have a lot of popups --- in particular, each comboboxes has one --- and we reposition them frequently.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: testcase

This testcase demonstrates the problem.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This patch basically fixes the issue. It improves the performance of the testcase on my machine (debug build!) by about 5x, from ~4700 to ~900 ms.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 232822 [details] [diff] [review]
fix

Makes sense.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Sorry Boris, I think we should go with this more conservative fix instead, which restricts the optimization to popups. I'm afraid of messing up WidgetToScreen calls applied to child widgets.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 232846 [details] [diff] [review]
fix

Ah, good catch.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: yet another try

Dear oh dear. The last one was borked because it didn't set "offset" for non-popup windows. hopefully this is it...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 232859 [details] [diff] [review]
yet another try

r+sr=bzbarsky, but are you sure you still trust my reviews?  :(

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in