[PATCH v3] Simplify current_add_equal loop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

* convert to infinite loop

         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
             dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                     common.base.channel_link);
             dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
-            while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
+            for (;;) {
+                if (!worker_ring_item || (dpi && dcc == dpi->dcc)) {
+                    break;
+                }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,


* split if

         while (worker_ring_item) {
             dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                     common.base.channel_link);
             dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
             for (;;) {
-                if (!worker_ring_item || (dpi && dcc == dpi->dcc)) {
+                if (!worker_ring_item) {
+                    break;
+                }
+                if (dpi && dcc == dpi->dcc) {
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);


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

             dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                     common.base.channel_link);
             dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
             for (;;) {
                 if (!worker_ring_item) {
-                    break;
+                    goto out_loop;
                 }
                 if (dpi && dcc == dpi->dcc) {
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
             }
             if (worker_ring_item) {
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
             }
         }
+out_loop:
         /* not sending other_drawable where possible */
         drawable_remove_from_pipes(other_drawable);

         drawable_unref(other_drawable);
         return TRUE;


* worker_ring_item is never null here

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


* dcc used is always a value from current worker_ring_item

         worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
-            dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
-                                    common.base.channel_link);
             dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
             for (;;) {
                 if (!worker_ring_item) {
                     goto out_loop;
                 }
+                dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
+                                        common.base.channel_link);
                 if (dpi && dcc == dpi->dcc) {
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
-                dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
-                                        common.base.channel_link);
             }

             if (dpi_ring_item) {
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
             }


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

                     goto out_loop;
                 }
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
                 if (dpi && dcc == dpi->dcc) {
+                    if (dpi_ring_item) {
+                        dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+                    }
+                    worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
+                                                 worker_ring_item);
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
             }
-
-            if (dpi_ring_item) {
-                dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-            }
-            worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                         worker_ring_item);
         }
 out_loop:
         /* not sending other_drawable where possible */
         drawable_remove_from_pipes(other_drawable);


* dpi is always a value computed from dpi_ring_item

         worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
-            dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
             for (;;) {
                 if (!worker_ring_item) {
                     goto out_loop;
                 }
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
+                dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 if (dpi && dcc == dpi->dcc) {
                     if (dpi_ring_item) {
                         dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                     }
                     worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,


* if dpi is not NULL even dpi_ring_item is not NULL

                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
                 dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 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);
                     worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                                  worker_ring_item);
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);


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

         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
             for (;;) {
                 if (!worker_ring_item) {
-                    goto out_loop;
+                    break;
                 }
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
                 dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 if (dpi && dcc == dpi->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                     worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                     break;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
             }
         }
-out_loop:
         /* not sending other_drawable where possible */
         drawable_remove_from_pipes(other_drawable);

         drawable_unref(other_drawable);
         return TRUE;


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

         worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
-            for (;;) {
-                if (!worker_ring_item) {
-                    break;
-                }
+            while (worker_ring_item) {
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
                 dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 if (dpi && dcc == dpi->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);


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

                 dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 if (dpi && dcc == dpi->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
                     worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                                  worker_ring_item);
-                    break;
+                    continue;
                 }
                 dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
             }


* reuse code inside loop

                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
                 dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
                 if (dpi && dcc == dpi->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                    worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                                 worker_ring_item);
-                    continue;
+                } else {
+                    dcc_prepend_drawable(dcc, drawable);
                 }
-                dcc_prepend_drawable(dcc, drawable);
                 worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
                                              worker_ring_item);
             }
         }


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

         worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
         while (worker_ring_item) {
-            while (worker_ring_item) {
-                dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
-                                        common.base.channel_link);
-                dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
-                if (dpi && dcc == dpi->dcc) {
-                    dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                } else {
-                    dcc_prepend_drawable(dcc, drawable);
-                }
-                worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                             worker_ring_item);
+            dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
+                                    common.base.channel_link);
+            dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
+            if (dpi && dcc == dpi->dcc) {
+                dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
+            } else {
+                dcc_prepend_drawable(dcc, drawable);
             }
+            worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
+                                         worker_ring_item);
         }
         /* not sending other_drawable where possible */
         drawable_remove_from_pipes(other_drawable);

         drawable_unref(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)

 switch (item->effect) {
 case QXL_EFFECT_REVERT_ON_DUP:
     if (is_same_drawable(drawable, other_drawable)) {

         DisplayChannelClient *dcc;
-        RedDrawablePipeItem *dpi;
         RingItem *worker_ring_item, *dpi_ring_item;

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

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


* use RING_FOREACH macro for loop

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

         /* sending the drawable to clients that already received
          * (or will receive) other_drawable */
-        worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
         dpi_ring_item = ring_get_head(&other_drawable->pipes);
         /* dpi contains a sublist of dcc's, ordered the same */
-        while (worker_ring_item) {
+        RING_FOREACH (worker_ring_item, &RED_CHANNEL(display)->clients) {
             dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                     common.base.channel_link);
             if (dpi_ring_item && dcc == SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base)->dcc) {
                 dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
             } else {
                 dcc_prepend_drawable(dcc, drawable);
             }
-            worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                         worker_ring_item);
         }
         /* not sending other_drawable where possible */
         drawable_remove_from_pipes(other_drawable);

         drawable_unref(other_drawable);

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
 server/display-channel.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

Changes from v2:
- update comment, some notes were missing.

diff --git a/server/display-channel.c b/server/display-channel.c
index 96a6f2f..6d459fe 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -464,7 +464,6 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
         if (is_same_drawable(drawable, other_drawable)) {
 
             DisplayChannelClient *dcc;
-            RedDrawablePipeItem *dpi;
             RingItem *worker_ring_item, *dpi_ring_item;
 
             other_drawable->refs++;
@@ -472,27 +471,15 @@ static int current_add_equal(DisplayChannel *display, DrawItem *item, TreeItem *
 
             /* sending the drawable to clients that already received
              * (or will receive) other_drawable */
-            worker_ring_item = ring_get_head(&RED_CHANNEL(display)->clients);
             dpi_ring_item = ring_get_head(&other_drawable->pipes);
             /* dpi contains a sublist of dcc's, ordered the same */
-            while (worker_ring_item) {
+            RING_FOREACH (worker_ring_item, &RED_CHANNEL(display)->clients) {
                 dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
                                         common.base.channel_link);
-                dpi = SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base);
-                while (worker_ring_item && (!dpi || dcc != dpi->dcc)) {
-                    dcc_prepend_drawable(dcc, drawable);
-                    worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                                 worker_ring_item);
-                    dcc = SPICE_CONTAINEROF(worker_ring_item, DisplayChannelClient,
-                                            common.base.channel_link);
-                }
-
-                if (dpi_ring_item) {
+                if (dpi_ring_item && dcc == SPICE_CONTAINEROF(dpi_ring_item, RedDrawablePipeItem, base)->dcc) {
                     dpi_ring_item = ring_next(&other_drawable->pipes, dpi_ring_item);
-                }
-                if (worker_ring_item) {
-                    worker_ring_item = ring_next(&RED_CHANNEL(display)->clients,
-                                                 worker_ring_item);
+                } else {
+                    dcc_prepend_drawable(dcc, drawable);
                 }
             }
             /* not sending other_drawable where possible */
-- 
2.7.4

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]