03 January, 2012

Avoid using busy-waiting or spinning technique

Busy-waiting, or in other words, spinning, happens when a process repeatedly executes a loop of code to see if a condition is true, while waiting for an event to occur. This means, that the processor will be waiting for the task to be finished in order to go on executing other tasks, which means, that in general this technique should be avoided, since it consumes time of the processor that could be used, wasting it on useless activity. The solution would be to use an asynchronous approach.

Thinking in asynchronous terms is hard though, synchronous API is simpler in many ways, so most of us tend to use it. It is also usually a quicker hack, but not because of this reason it is the best option.

In order to let Cheese users launch several nautilus-sendto dialogs, to send several pictures each time, and change the form of the cursor from when Cheese was busy with nautilus-sendto to when it was not and all of these without blocking the UI, I needed to take an asynchronous approach. I do not have experience with it, so I of course did it the "easy way", spinning the loop (spin_event_loop()) and this was the result (please, take into account that I removed all the code that was not necessary for this explanation):
public void files_share() {
    try {
       main_window.set_busy_cursor();
       Pid child_pid;
       Process.spawn_async("/", argv, null SpawnFlags.SEARCH_PATH, null, out child_pid);
       main_window.set_normal_cursor();
    }
public void set_busy_cursor() {
   get_window().set_cursor(new Gdk.Cursor(Gdk.CursorType.WATCH));
   spin_event_loop();
}
public void set_normal_cursor() {
   get_window().set_cursor(new Gdk.Cursor(Gdk.CursorType.LEFT_PTR));
   spin_event_loop();
}
public bool spin_event_loop() { 

   while (Gtk.events_pending()) // Busy-waiting
     {...}
} 
After submitting my first patch with this code, Dave was the one who made the review, and he suggested me to not use the spinning technique. Now it is obvious after reading the above mentioned reasons why :P. At first I was a little bit "afraid" of using asynchronous API. It looked completely akward to me, but then I also discovered through him, that GIO and GLib have some nice API to make asynchronous operations simpler. GIO offers GSimpleAsyncResult, which I personally did not find specially pretty, but still very useful. Sushi uses it and has some nice examples in its code. On the other hand, GLib has g_child_watch_add() (ChildWatch.add()), which sets a function to be called (child_finished()) when a child exits, and was the best approach for my case and the one I decided to replace the spinning technique with:
public void files_share () {
   try {
      Pid child_pid;
      if (Process.spawn_async("/", argv, null,
                                             SpawnFlags.SEARCH_PATH | SpawnFlags.DO_NOT_REAP_CHILD,
                                             null, out child_pid))
      {
         ChildWatch.add (child_pid, child_finished);
         if (num_children == 1)
            window.get_window().set_cursor(new Gdk.Cursor(Gdk.CursorType.WATCH));
      }
   }
}
void child_finished (Pid pid, int status) {
   Process.close_pid(pid);
   if (num_children == 0)
      window.get_window().set_cursor(new Gdk.Cursor(Gdk.CursorType.LEFT_PTR));
}
All of these has been totally new for me and I am very happy I learned some bits of it. It definitely improved the code a lot!

Hopefully, all these changes will be merged to master very soon. I will keep you informed!

2 comments:

  1. Since you're using Vala, there's a super simple way to write an asynchronous method.

    Of course, that won't help if you have to use a function someone else wrote which isn't compatible with Vala's async feature, but it's something handy to keep in mind.

    ReplyDelete
  2. Assuming that the while loop spins the event loop (as the name suggests), this isn't busy-looping.

    You're simply creating a recursive event-loop manually. Which has some caveats which make it a bad idea, but it's a still a very different thing from busy-looping :-)

    ReplyDelete