[Spice-devel] [PATCH v4] Simplify current_add_equal loop

Frediano Ziglio fziglio at redhat.com
Wed May 25 08:34:26 UTC 2016


With some simple steps I simplified the internal loop of this
function

* convert to infinite loop

     while (link) {
         dcc = link->data;
         dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
-        while (link && (!dpi || dcc != dpi->dcc)) {
+        for (;;) {
+            if (!link || (dpi && dcc == dpi->dcc)) {
+                break;
+            }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;
             if (link)

* split if

         dcc = link->data;
         dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
         for (;;) {
-            if (!link || (dpi && dcc == dpi->dcc)) {
+            if (!link) {
+                break;
+            }
+            if (dpi && dcc == dpi->dcc) {
                 break;
             }
             dcc_prepend_drawable(dcc, drawable);

* the link condition make this case terminate loop
  dpi_ring_item is changed but ignored

         dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
         for (;;) {
             if (!link) {
-                break;
+                goto out_loop;
             }
             if (dpi && dcc == dpi->dcc) {
                 break;
             }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;
             if (link)
                 dcc = link->data;
         }

         if (dpi_ring_item) {
             dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
         }
         if (link) {
             link = link->next;
         }
     }
+out_loop:
     /* not sending other_drawable where possible */
     drawable_remove_from_pipes(other_drawable);

* link is never null here

         if (dpi_ring_item) {
             dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
         }
-        if (link) {
-            link = link->next;
-        }
+        link = link->next;
     }
 out_loop:
     /* not sending other_drawable where possible */

* dcc used is always a value from current link

     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
     while (link) {
-        dcc = link->data;
         dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
         for (;;) {
             if (!link) {
                 goto out_loop;
             }
+            dcc = link->data;
             if (dpi && dcc == dpi->dcc) {
                 break;
             }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;
-            if (link)
-                dcc = link->data;
         }

         if (dpi_ring_item) {

* move code inside if
  these lines are only executed then the condition are satisfied

             }
             dcc = link->data;
             if (dpi && dcc == dpi->dcc) {
+                if (dpi_ring_item) {
+                    dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+                }
+                link = link->next;
                 break;
             }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;
         }
-
-        if (dpi_ring_item) {
-            dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-        }
-        link = link->next;
     }
 out_loop:
     /* not sending other_drawable where possible */

* dpi is always a value computed from dpi_ring_item

     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
     while (link) {
-        dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
         for (;;) {
             if (!link) {
                 goto out_loop;
             }
             dcc = link->data;
+            dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {
                 if (dpi_ring_item) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);

* if dpi is not NULL even dpi_ring_item is not NULL

             dcc = link->data;
             dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {
-                if (dpi_ring_item) {
-                    dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                }
+                dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                 link = link->next;
                 break;
             }

* if link is NULL jumping outside loop will check variable and exit
  The outer loop will exit too.

     while (link) {
         for (;;) {
             if (!link) {
-                goto out_loop;
+                break;
             }
             dcc = link->data;
             dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                 link = link->next;
                 break;
             }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;
         }
     }
-out_loop:
     /* not sending other_drawable where possible */
     drawable_remove_from_pipes(other_drawable);

* use while again
  Convert from for(;;) { if () break; ... } to while

     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
     while (link) {
-        for (;;) {
-            if (!link) {
-                break;
-            }
+        while (link) {
             dcc = link->data;
             dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {

* convert break to continue
  Both jump to check worker_ring_item (internal or external loops)

             dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                 link = link->next;
-                break;
+                continue;
             }
             dcc_prepend_drawable(dcc, drawable);
             link = link->next;

* reuse code inside loop

             dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
             if (dpi && dcc == dpi->dcc) {
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                link = link->next;
-                continue;
+            } else {
+                dcc_prepend_drawable(dcc, drawable);
             }
-            dcc_prepend_drawable(dcc, drawable);
             link = link->next;
         }
     }

* join the two loop
  They tested the same condition, no break in inside loop (which would
  exit all loops)

     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
     while (link) {
-        while (link) {
-            dcc = link->data;
-            dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
-            if (dpi && dcc == dpi->dcc) {
-                dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-            } else {
-                dcc_prepend_drawable(dcc, drawable);
-            }
-            link = link->next;
+        dcc = link->data;
+        dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
+        if (dpi && dcc == dpi->dcc) {
+            dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+        } else {
+            dcc_prepend_drawable(dcc, drawable);
         }
+        link = link->next;
     }
     /* not sending other_drawable where possible */
     drawable_remove_from_pipes(other_drawable);

* inline dpi computation
  Actually not exactly the same as this fix a bug if base is not the
  first element (in this case if dpi_ring_item is NULL dpi is not)

 if (is_same_drawable(drawable, other_drawable)) {

     DisplayChannelClient *dcc;
-    RedDrawablePipeItem *dpi;
     RingItem *dpi_ring_item;
     GList *link;

     other_drawable->refs++;
     current_remove_drawable(display, other_drawable);

     /* sending the drawable to clients that already received
      * (or will receive) other_drawable */
     link = RED_CHANNEL(display)->clients;
     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
     while (link) {
         dcc = link->data;
-        dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
-        if (dpi && dcc == dpi->dcc) {
+        if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) {
             dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
         } else {
             dcc_prepend_drawable(dcc, drawable);

* use FOREACH_CLIENT macro for loop

     DisplayChannelClient *dcc;
     RingItem *dpi_ring_item;
-    GList *link;
+    GList *link, *next;

     other_drawable->refs++;
     current_remove_drawable(display, other_drawable);

     /* sending the drawable to clients that already received
      * (or will receive) other_drawable */
-    link = RED_CHANNEL(display)->clients;
     dpi_ring_item = ring_get_head(&other_drawable->pipes);
     /* dpi contains a sublist of dcc's, ordered the same */
-    while (link) {
-        dcc = link->data;
+    FOREACH_CLIENT(display, link, next, dcc) {
         if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) {
             dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
         } else {
             dcc_prepend_drawable(dcc, drawable);
         }
-        link = link->next;
     }
     /* not sending other_drawable where possible */
     drawable_remove_from_pipes(other_drawable);

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/display-channel.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

Changes from v3:
- rebased on master.

diff --git a/server/display-channel.c b/server/display-channel.c
index 13be947..cfa182c 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -466,33 +466,21 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
         if (is_same_drawable(drawable, other_drawable)) {
 
             DisplayChannelClient *dcc;
-            RedDrawablePipeItem *dpi;
             RingItem *dpi_ring_item;
-            GList *link;
+            GList *link, *next;
 
             other_drawable->refs++;
             current_remove_drawable(display, other_drawable);
 
             /* sending the drawable to clients that already received
              * (or will receive) other_drawable */
-            link = RED_CHANNEL(display)->clients;
             dpi_ring_item = ring_get_head(&other_drawable->pipes);
             /* dpi contains a sublist of dcc's, ordered the same */
-            while (link) {
-                dcc = link->data;
-                dpi = SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item);
-                while (link && (!dpi || dcc != dpi->dcc)) {
-                    dcc_prepend_drawable(dcc, drawable);
-                    link = link->next;
-                    if (link)
-                        dcc = link->data;
-                }
-
-                if (dpi_ring_item) {
+            FOREACH_CLIENT(display, link, next, dcc) {
+                if (dpi_ring_item && dcc == SPICE_UPCAST(RedDrawablePipeItem, dpi_ring_item)->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                }
-                if (link) {
-                    link = link->next;
+                } else {
+                    dcc_prepend_drawable(dcc, drawable);
                 }
             }
             /* not sending other_drawable where possible */
-- 
2.7.4



More information about the Spice-devel mailing list