! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Improve keyboard-settings startup speed
Status:
RESOLVED: FIXED
Severity:
enhancement
Product:
Xfce4-settings
Component:
Keyboard Settings

Comments

Description Martin Pitt 2010-08-24 08:31:22 CEST
Created attachment 3087 
performance trace with 4.7.1

We noticed that xfce4-keyboard-settings takes about two seconds to start on a slow embedded system. Using the approach from [1] I added some profiling points to main() and the dialog setup in xfce_keyboard_settings_constructed(), which showed that the construction of the keyboard model combo box took one second, half of the startup time.

Since the keyboard layout tab is not the tab shown by default, and is usually even grayed out, I think it is acceptable to take out this part and move it into a g_idle_add().

[1] http://people.gnome.org/~federico/news-2006-03.html#login-time-2
Comment 1 Martin Pitt 2010-08-24 08:32:48 CEST
Created attachment 3088 
performance trace with the patch applied

With the patch, the trace now looks like this, and xfce4-keyboard-settings now starts up in just under a second. I tested that the layout combo box still works fine.
Comment 2 Martin Pitt 2010-08-24 08:36:27 CEST
Created attachment 3089 
patch against git head

Finally, the patch. Thanks for considering!
Comment 3 Nick Schermer editbugs 2010-08-24 16:11:09 CEST
What happens if you move gtk_combo_box_set_model() after the foreach call?
Comment 4 Martin Pitt 2010-08-25 13:44:50 CEST
Nick,

if you do that, you'll get tons of


(xfce4-keyboard-settings:8956): Gtk-CRITICAL **: gtk_list_store_set_valist: assertion `GTK_IS_LIST_STORE (list_store)' failed

(xfce4-keyboard-settings:8956): Gtk-CRITICAL **: gtk_list_store_append: assertion `GTK_IS_LIST_STORE (list_store)' failed

and an empty combobox. That's because xfce_keyboard_settings_add_model_to_combo() calls gtk_combo_box_get_model(), which isn't set yet at that time. So we'd need to add the local list_store variable to the settings struct, to pass it around.

Why do you ask, do you think it'd make any performance difference if you initialize the list store before assigning it to the combo box? (that'd weird; unless you mean that this would save us a hundred gtk_combo_box_get_model() calls, but I suppose they are cheap (and presumably compiler optimized) compared to the gtk_builder_get_object() call which we do in each iteration.
Comment 5 Nick Schermer editbugs 2010-08-25 16:47:01 CEST
Created attachment 3091 
Connect to widget after model is filled

I mean like this. GtkComboBox responds to each insert (shouldn't be a lot tho) and the object lookups are also unneeded. If this is hardly an improvement we can go with the idle, but I'm not a huge fan of idle stuff in startup interfaces (idle still runs in the same thread, so if shows faster, but the interface is still blocked a little while after it is shown) and I'd say loading the xkl config would take some time, so if we go idle; do all the xkl tasks in the idle.
Comment 6 Martin Pitt 2010-08-30 12:50:31 CEST
Created attachment 3102 
trace before "anothertry.patch"

Indeed this reduces the time to initialize the models combo box from 1 s to ~ 0.3 seconds, nice job! So it seems the combo box does some nontrivial things each time you change its model, even if it's not displayed.

I used a very reduced tracing instrumentation this time, so for reference I attach the before/after your patch graphs again.

Thanks!
Comment 7 Martin Pitt 2010-08-30 12:50:53 CEST
Created attachment 3103 
trace after "anothertry.patch"
Comment 8 Nick Schermer editbugs 2010-09-03 18:51:21 CEST
Ok will commit my patch then (rev f254a7a), since it solves the problem instead of 'hiding' it in an idle. Thanks a lot for testing this, I really appreciate it people look at the code this way.

Bug #6661

Reported by:
Martin Pitt
Reported on: 2010-08-24
Last modified on: 2010-09-03

People

Assignee:
Jannis Pohlmann
CC List:
3 users

Version

Attachments

performance trace with 4.7.1 (108.88 KB, image/png)
2010-08-24 08:31 CEST , Martin Pitt
no flags
performance trace with the patch applied (117.60 KB, image/png)
2010-08-24 08:32 CEST , Martin Pitt
no flags
patch against git head (3.32 KB, patch)
2010-08-24 08:36 CEST , Martin Pitt
no flags
Connect to widget after model is filled (4.65 KB, patch)
2010-08-25 16:47 CEST , Nick Schermer
no flags
trace before "anothertry.patch" (27.61 KB, image/png)
2010-08-30 12:50 CEST , Martin Pitt
no flags
trace after "anothertry.patch" (24.49 KB, image/png)
2010-08-30 12:50 CEST , Martin Pitt
no flags

Additional information