Patch and changelog (for libbonoboui).
After exploring many debugging outputs and gdb stack traces I realized that an accelerator was changed whenever the node of the xml tree that represented a specific menu widget was marked as “dirty”. In the process of merging the several xml trees a node could be set dirty depending on the command that represent. Some commands need to be synchronized; this sync is transferred to the node, for example in impl_bonobo_ui_sync_menu_state. But the problem that this function had was the chaotic addition of accelerators which made an expensive growth of structures, memory leaks included.
The patch respects the fact that we can have a widget with multiple accelerators and that the same accelerator can be added many times in the same accelerator group. The code only avoid the accelerator addition if it has been added previously to the same widget (which is the common case).
A chart with the difference of the old and new behaviour of component mail switching (200 switches).
After each switch, evo sleep, for avoiding the linux scheduler interference. Other charts didn’t take into account that, but it doesn’t matter, because the effects and conclusions were the same. For example, the following chart represents a massive 200-switching. The old version never stops growing and the new one become stable in a specific threshold, determined by the end of an epoch in the Linux scheduler (probably evo should be at the end of the runqueue, but starvation doesn’t happen).
* Edited * I’ve changed the patch because it had a little problem of memory leaking. Thanks Benoît Dejean for your remark!
#pragma omp parallel private (blog) 
Alex Jones said
Good work, Cecilia! 🙂
Emmanuele said
great work indeed; am I wrong or this should also improve performances for panel applets and other bonobo users out there?
Raphael Bosshard said
Woot! Nice Work!
andre said
this is great, so much love has been spent on evolution for the last 6 months, especially with regard to speed improvements and memory fixes.
so a big thanks to cecilia, and to everybody in the gnome community!
Andreas said
Wow! Great job!
Kurt McKee said
Excellent work! Thank you!
Ken said
Excellent…
This is only a fix for Evo, right? Setting accelerators in Gtk+ is still (relatively) slow, it sounds like.
Matthew W. S. Bell said
Nice.
Roberto said
like u, contact me.
Benoît Dejean said
isn’t your patch leaking ?
+ if (n_entries)
+ for (l_tmp = l_closures; l_tmp; l_tmp = l_tmp->next)
+ for (i=0; idata)
+ return TRUE;
+
+ g_list_free (l_closures);
+
+ return FALSE;
+}
you forgot to free l_closures, didn’t you ?
pedro said
Amazing work, Cecilia. Congratulations!
Lee Willis said
Wouldn’t this be better going into GTK itself (Therefore fixing it for any other cases where this could occur) as well?
I can’t think of any reason why it would ever be valid to add the same accelerator to a widget multiple times?
Was there a particular reason this got fixed in bonobo, and not lower down in GTK?
m3gumi said
Thank you all for your comments!
I hope this patch improve performace on applications using bonoboui.
Benoît Dejean: I free l_closures at the end of the function.
Lee Willis: I found the patch easier to implement in bonobo and I didn’t know if I should forbid the addition of the same accelerator in GTK, though it looks odd, I know.
Benoît Dejean said
> Benoît Dejean: I free l_closures at the end of the function.
Not if you return TRUE;
m3gumi said
Benoît Dejean:
Of course, you’re right! Thanks, I’ll correct my stupid mistake.