Lazy loading

Little post about a useful design pattern in GLib and GTK, written down after a question on the #gtk+ channel

While I’m not Philip and I won’t go as far as talking about this in public, I wrote down a simple design pattern for lazy loading stuff from a list into a GtkTreeView without blocking your interface or using threads.

Lazy loading using the main loop.

Roadmap

With the incoming release of Gnome 2.13.90, we are approaching the 2.14 version of Gnome. Now, features are frozen, and soon even changing the UI will require at least two approvals (one from the GUP and one from the release team); so, while we still have a full month for fixing bugs, I think it’s time for some little thinking and planning ahead about the next release cycle of gnome-utils.

The biggest job for the 2.15/2.16 cycle will be the cleaning up of the System Log Viewer. The code-base is a bit messy, but thanks to the great work of Vincent we’ve avoided the Design by Accretion Syndrome; still, even a white space consistency patch would be in order: some lines have a (horrid) three-space indentation, while some other have a tab indentation. So, I’d move everything to a tab-based indentation. The other “cosmetic” clean up is the removal of the dead and/or obsolete calls, the usage of a naming scheme for the classes and functions, and a switch to typed objects (e.g. boxed types and GObject-based types). I’ve already begun some attempt at cleaning up the code base, but this will most likely require to land in a new branch as soon as we release gnome-utils 2.14.

The other job is the implementation of a couple of transport methods for Dictionary, namely the HTTP-based transport, which should allow the connection to a web-based dictionary service; the file-based transport, which should allow the querying of locally available dictionaries; the StarDict transport, similar to the file-based one, but including a C parser for the StarDict format. Regarding this format, I’ve had a look at the C++ library and I seriously think that before releasing some software under an open source licence, a serious check on the code style should be in order; I don’t like C++, I think it’s inherently inefficient and messy, but something like this:

sd-lib-cpp
whitespace horror in lib.cpp

should really be closed source software – in order to avoid programmers throwing themselves out of the window after having had a look at it.

One more job would be the re-working of the gfloppy utility, with the addition of the ability to format every removable media (floppies, USB sticks, CD/DVD RW, etc). This will require some updates inside HAL – namely, the volume formatting and partitioning support. I think that, while floppies are becoming more and more rare these days, gfloppy might be reborn into something very useful again.

Finally, the last job for the 2.15/2.16 cycle should be the update of the Screenshot utility. First of all, I’d like to close bug #325708, but the whole bug list should be triaged and updated.

Obviously, I can’t promise that everything will be ready in time for 2.16. As always, patches that will make this happen faster are welcome.

Broken

The next release of gnome-utils, 2.13.90, will make libgdict adhere to the API/ABI freeze, even if it’s not part of the Gnome Developer Platform but only of the Desktop.

The freeze had been in effect since the last release (January 18th), and I planned not to change libgdict API; unfortunately, when writing the support for the document_font_name GConf key, which was introduced in Gnome 2.12 and that should be honoured by any application showing arbitrarily long texts to the user, I noticed that a call to gtk_widget_modify_font to a GtkContainer does not propagate to the containers’s children. The widget the Dictionary uses to display the text of the definitions is, in fact, a composite widget (a GtkVBox) as it needs to hold the text display and the find bar; the inner widgets are held inside a private structure, and are not visible to the outside.

If I wanted to work around this, I would have written something like this function inside the code of the libgdict users:

static void
gtk_container_modify_font_children (GtkContainer *container,
				    const gchar  *font_name)
{
  GList *children, *l;
  PangoFontDescription *font_desc;

  g_return_if_fail (GTK_IS_CONTAINER (container));

  font_desc = NULL;
  if (font_name)
    {
      font_desc = pango_font_description_from_string (font_name);
      g_return_if_fail (font_desc != NULL);
    }

  children = gtk_container_get_children (container);
  for (l = children; l != NULL; l = l->next)
     {
        GtkWidget *widget = GTK_WIDGET (l->data);

	gtk_widget_modify_font (widget, font_desc);
     }

  g_list_free (children);

  if (font_desc)
    pango_font_description_free (font_desc);
}

But it would not have worked; I wanted to change the font of the GtkTextView widget inside the GdictDefbox, while the function above would have changed font for all internal widgets – including the find pane.

Assigning a name to the GtkTextView widget, say “text-display”, by using the gtk_widget_set_name function, and changing the code of the inner loop to something like this:

...
  for (l = children; l != NULL; l = l->next)
     {
        GtkWidget *widget = GTK_WIDGET (l->data);
	const gchar *name = gtk_widget_get_name (widget);

	if (strcmp (name, "text-display") == 0)
          gtk_widget_modify_font (widget, font_desc);
     }
...

I would have avoided the API breakage inside libgdict – but I would also have created a performance bottleneck, since I transformed a constant time operation into a linear time one (from an assignment to a list walk) – plus, I don’t know what happens into gtk_widget_modify_font; also, I would have created a documentation issue, since I now would have to document the widget’s name and hope that nobody would ever have the urge to change the GdictDefbox font – at least, not after having had lunch.

Giving a name to the inner children of a composite widget is always a good practice – it makes handling these kind of situations easier; but between an hackish approach and a breakage of an API freeze, I would rather choose the latter. Hacks tend to get sticky, and you get a design by accretion if you let them stick around enough.

So, I opten for adding a new property to the GdictDefbox widget, called font-name, and its two accessor functions:

G_CONST_RETURN gchar *gdict_defbox_get_font_name (GdictDefbox *defbox);
void                  gdict_defbox_set_font_name (GdictDefbox *defbox,
						  const gchar *font_name);

Which are just proxies for the gtk_widget_modify_font function. Since nobody uses libgdict (it’s been out there only for the folks using Ubuntu or jhbuild), the breakage is really minimal; nevertheless, I feel a bit guilty for not having tested this stuff before, in time for the freeze.

There And Back Again

Again at home, after a week in Berlin.

The city is wonderful – now I understand why Marta loves it that much: the place is gorgeous, the people is warm and they really make you feel at home. Me and Marta were both a bit sad to leave – but we plan to return there as soon as possible, maybe even in summer, even though with the GUADEC 2006 moved at the end of June we’d already have our summer holidays covered. Time will tell.

We’ve done a ton of photos – but the last day’s worth of them (mostly about and from the dome on top of the Reichstag/Bundestag) were eaten by F-Spot; it was an older version of it, and I did the stupidest thing by deleting them from my card too, but F-Spot shouldn’t lie about having done the import and then really having finished just the thumbnailing.

While in Berlin we’ve met for a couple of hours Torsten Schoenfeld, another gtk2-perl hacker – well, he is the gtk2-perl Release Master and the Test Suite God, other than being more than Just Another Perl Hacker; he is a real pleasure to talk with and a great guy. If you ever come to Italy, you’re up for a beer, or more than one. ;-)

Now that the winter holidays are really over, let’s get back to work.

The gnome-utils release went fine (even though there’s a typo in the NEWS file I’ve submitted – dang!), and a bunch of bugs have been filed in Bugzilla. Keep them flowing, so I can know what doesn’t work. I’m off to add the window-size-saved-across-session feature that was added to the search tool, and to make the icon in the applet become a toggle button, instead of a plain icon.

Another project I’m working on is the build environment for the Glib Perl module and Glib-based Perl extensions; I began looking at Module::Build while in Berlin, but in the end, I came up with another solution – which will be easier to port to Module::Build later on, when that module enters the standard Perl base distribution. Anyway, I’ll talk about this issue later in a (lengthy) post, so stay tuned.

There are a bunch of fixes due for the libegg/recentchooser code, the main one being the sorting functions duplicated from the RecentManager object into the RecentChooser object; in the end, I’d like to remove all the sorting/filtering stuff from the RecentManager, and let all the UI built upon the RecentChooser interface provide their own sorting/filtering stuff. This would make the RecentManager object a thin layer upon the BookmarkFile object, and would really make things easier to be included into the GTK library. In the end, all the sorting a filtering is something that has to do with the display of the data that the RecentManager holds, so they do not belong into the manager itself. Other than this fix, I’ll begin working on a patch for the BookmarkFile object in order for it to land inside Glib, and a patch for the FileSystem object to use it for its bookmarks.

Leaking

Yesterday, while I was at university waiting for a seminar about network security, I was on #gnome-it, and pbor asked me to look at a leak in EggRecentModel. I began wading through the code, but I didn’t found the leakage in time before having to leave.

In the evening, after I got back home, Marta went to her church choir practice, and I looked at recent-files/egg-recent-model.c using Valgrind (and practicing with it, since I’m not that good with this tool); using the function chain reported by valgrind, I noticed that the leak was inside the logic used by egg_recent_model_filter. This function scans the list of EggRecentItem objects built from the on-disk storage and filter out using the three pre-defined filter functions (while my RecentManager object supports only custom filtering function, and the RecentChooser implementations might filter the recently used resources list using a more complex RecentFilter object). The problem is that it used the infamous GList scan using while():

  while (list) {
    SomeData *obj = l->data;

    do something with the object

    list = list->next;
  }

This code is perfectly valid: it walks the list by exhausting it; that is: the list pointer reaches the end of the list once it reaches the end of the while. Valid code does not mean right code, though. Suppose you want to move the items of a list inside another list. By using the code above, you would have:

  GList *newlist;

  /* init the target list */
  newlist = NULL;
  while (list) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);

    list = list->next;
  }

This code is also perfectly valid for filtering a list. Suppose, now, that you want to destroy the original list and return the filtered copy; you’ll have to free the GList using g_list_free() at the end of the while cycle:

  GList *newlist;

  /* init the target list */
  newlist = NULL;
  while (list) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);

    list = list->next;
  }

  g_list_free (list);

  return newlist;

But we said above that the list is walked by exhausting it; hence, the list pointer is now NULL (the exit condition checked by the while cycle). Thus you are leaking the old list; to avoid this leakage, you must walk the list keeping a pointer to the list start. In order to do so, the right thing to do is to use an iterator pointer, and a for() cycle to keep the cycle compact and avoid messing up with the list = list->next assignments on every cycle break condition:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);
  }

  g_list_free (list);
  return newlist;

This code does not leak the old list, but it leaks the data that was not moved to the new list; if the function to free the data is called some_list_free, the code above becomes:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_append (newlist, obj);
    else
      some_data_free (obj);
  }

  g_list_free (list);
  return newlist;

As a performance sidenote: if the list is long, you might consider using g_list_prepend, since it’s a constant time operation, while g_list_append walks the list; just remember to return the reversed new list:

  GList *newlist, *l;

  for (l = list, newlist = NULL; l; l = l->next) {
    SomeData *obj = l->data;

    if (some_condition)
      newlist = g_list_prepend (newlist, obj);
    else
      some_data_free (obj);
  }

  g_list_free (list);
  return g_list_reverse (newlist);

I had noticed the leak around 10:30pm and began fixing it, but I had to pick up Marta and both returned home at 11:30pm, when we went straight to bed and fell asleep almost immediately. This morning, I found bug #323002 awaiting, with a fix for the first part of the leak (thanks to Paolo). Now, libegg/recent-files HEAD has the leak fully plugged. Subsequent runs of valgrind showed no mor leaks related to the recent-files.

Now, I’m going to check my libegg/recentchooser and my libegg/bookmarkfile code for leaks.

[]
[]
[]

Dictionary Applet/3

In case you’ve been wondering if I had disappeared or something…

I’m in the middle of a session of exams, this week (four of them, to be precise), and I’ve had to cut down hacking time; nevertheless, I’ve been able to commit a set of fixes for the RecentChooser/BookmarkFile code and wrap up a new version of the desktop-bookmark storage spec.

GNOME Dictionary

I’ve also started the rewriting of the low-level implementation of the dictionary protocol client object for the GNOME Dictionary application. I’ve tried one last time to salvage the sane bits of the old code, but it was practically useless. Instead, I designed a new, object-oriented, approach. It’s been three years since I’ve done some networking with C, and I had to do some catch-up in order to write some decent code.

The new client object is called (with a certain lack of creativity) GDictContext; you create a new context by using the gdict_context_new() function, and since it inherits from GObject, all you have to do to destroy it is to call g_object_unref() on it.

All the querying and replying is done asynchronously; but instead of having callback functions for each command, I preferred a more GTK-like approach; now, you’ll have to connect to signals, which are emitted by the context object when something comes along the wire; thus, creating a client for the dictionary protocol is just a matter of a few lines of code:

static void
on_connect (GDictContext *context
            gpointer      user_data)
{
  GError *error = NULL;

  gdict_context_define (context, NULL, "penguin", &error);
  if (error)
    {
      g_warning ("Error: %s", error->message);
      g_error_free (error);

      g_main_loop_quit (main_loop);
    }
}

static void
on_def_found (GDictContext    *context,
              GDictDefinition *definition,
	      gpointer         user_data)
{
  g_print ("Definition for '%s' (from: %s)\n%s\n",
           gdict_definition_get_word (definition),
	   gdict_definition_get_from (definition),
	   gdict_definition_get_definition (definition));
}

...

  GDictContext *context;

  main_loop = g_main_loop_new (NULL, FALSE);

  context = gdict_context_new ("dict.org", NULL);
  g_signal_connect (context, "connect",
                    G_CALLBACK (on_connect), NULL);
  g_signal_connect (context, "definition-found",
                    G_CALLBACK (on_def_found), NULL);

  gdict_context_connect (context, &connect_error);
  if (connect_error)
    {
      g_warning ("Error: %s", connect_error->message);
      g_error_free (connect_error);
      g_object_unref (G_OBJECT (context));

      return -1;
    }

  g_main_loop_run (main_loop);

  g_object_unref (G_OBJECT (context));

  return 0;

As you can see, no more silly creation of commands and contextes, using redundant, badly defined or over-designed API (which, by the way, badly leaks strings and objects): just lean and mean, OO, event-driven code; I plan to make this a shared library (it already is, albeit it’s not exported), so every public function comes with documentation and follows the same style rules of the other platform libraries; I could even add bindings to it, and write the whole application/applet in an high-level language, like Perl or Python, and the effort would really be minimal.

Right now, the code is still flaky on the response parsing; I plan to make it more robust as soon as the week ends, and I’m free to use more time on it. After that, I’ll attack the UI side – and get a hold on the messy situation of the definition box, the speller tab and the preferences window. This should really take less than the low-level stuff: the code is a lot saner, even though requiring much love. The self-imposed deadline of late November/early December should be fully respected.

Glib::KeyFile

I began working on a perl binding for the key file parser inside Glib. I have an implementation almost ready, albeit not usable right now.

I’d like to translate a key file structure to the more perlish equivalent of an associative array; that is, I want to translate this:

  [Section]
  # comment
  stringkey=astring
  intkey=42
  boolkey=false

Into this:

  print $keyfile->{Section}->{comment}; # prints "comment"
  print $keyfile->{Section}->{intkey}; # prints "42"
  ...

While skimming through the gkeyfile.[ch] files, I’ve also noticed that you can’t create a key file in code, even though the infrastructure is in place (you can create an empty object, and you can remove groups and keys from a filled object). Thus I’ve created a bug inside Gnome’s bugzilla with a proper patch to expose the addition of groups and keys (here). I’ll notify the gtk-devel mailing list, and see if someone wants to review (and apply) my patch.

On the language binding friendlyness side, I’ve also made a patch that transforms the GKeyFile structure in a reference counted object, but I won’t submit it, since it seems that no structs inside Glib core are refcounted. Pity, but I learned something new.