[Freeciv-Dev] Re: (PR#10180) [Patch] Optimize MAX(), MIN() calls

["Marko Lindqvist" <marko.lindqvist@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10180 >

Jason Short wrote:
> 
>>[marko.lindqvist@xxxxxxxxxxx - Sat Sep 18 19:34:26 2004]:
> 
>>  I had to change macro SQSIZE into inline function get_sqsize() in 
>>order to use temporary variable.
> 
> This part is kindof ugly.  This shouldn't be an inline function and
> should probably just be a regular function (rather than a macro).  You
> can update the patch to fix this or just drop it if you like.

  I made it regular function. Patch updated against HEAD but untested.


  - Caz

diff -Nurd -X.diff_ignore freeciv/client/messagewin_common.c 
freeciv/client/messagewin_common.c
--- freeciv/client/messagewin_common.c  2004-11-20 18:40:19.750000000 +0200
+++ freeciv/client/messagewin_common.c  2004-11-20 18:44:34.984375000 +0200
@@ -113,7 +113,8 @@
   const char *game_prefix2 = _("Game: ");
   size_t gp_len1 = strlen(game_prefix1);
   size_t gp_len2 = strlen(game_prefix2);
-  char *s = fc_malloc(MAX(strlen(message), min_msg_len) + 1);
+  size_t msg_len = strlen(message);
+  char *s = fc_malloc(MAX(msg_len, min_msg_len) + 1);
   int i, nspc;
 
   change = TRUE;
diff -Nurd -X.diff_ignore freeciv/common/map.c freeciv/common/map.c
--- freeciv/common/map.c        2004-11-20 18:40:21.718750000 +0200
+++ freeciv/common/map.c        2004-11-20 18:44:35.031250000 +0200
@@ -594,6 +594,8 @@
 ****************************************************************************/
 int map_vector_to_real_distance(int dx, int dy)
 {
+  int absdx = abs(dx);
+  int absdy = abs(dy);
   if (topo_has_flag(TF_HEX)) {
     if (topo_has_flag(TF_ISO)) {
       /* Iso-hex: you can't move NE or SW. */
@@ -601,10 +603,10 @@
          || (dx > 0 && dy < 0)) {
        /* Diagonal moves in this direction aren't allowed, so it will take
         * the full number of moves. */
-       return abs(dx) + abs(dy);
+        return absdx + absdy;
       } else {
        /* Diagonal moves in this direction *are* allowed. */
-       return MAX(abs(dx), abs(dy));
+        return MAX(absdx, absdy);
       }
     } else {
       /* Hex: you can't move SE or NW. */
@@ -612,14 +614,14 @@
          || (dx < 0 && dy < 0)) {
        /* Diagonal moves in this direction aren't allowed, so it will take
         * the full number of moves. */
-       return abs(dx) + abs(dy);
+       return absdx + absdy;
       } else {
        /* Diagonal moves in this direction *are* allowed. */
-       return MAX(abs(dx), abs(dy));
+        return MAX(absdx, absdy);
       }
     }
   } else {
-    return MAX(abs(dx), abs(dy));
+    return MAX(absdx, absdy);
   }
 }
 
diff -Nurd -X.diff_ignore freeciv/common/tech.c freeciv/common/tech.c
--- freeciv/common/tech.c       2004-11-20 18:40:22.093750000 +0200
+++ freeciv/common/tech.c       2004-11-20 18:44:35.031250000 +0200
@@ -554,10 +554,10 @@
   } tech_type_iterate_end;
 
   tech_type_iterate(tech) {
-    techcoststyle1[tech] = MAX((advances[tech].num_reqs + 1)
-                               * sqrt(advances[tech].num_reqs + 1)
-                               * (game.researchcost / 2),
-                               game.researchcost);
+    int style1_cost = (advances[tech].num_reqs + 1)
+                      * sqrt(advances[tech].num_reqs + 1)
+                      * (game.researchcost / 2);
+    techcoststyle1[tech] = MAX(style1_cost, game.researchcost);
   } tech_type_iterate_end;
 }
 
diff -Nurd -X.diff_ignore freeciv/server/generator/mapgen.c 
freeciv/server/generator/mapgen.c
--- freeciv/server/generator/mapgen.c   2004-11-20 18:41:02.250000000 +0200
+++ freeciv/server/generator/mapgen.c   2004-11-20 19:01:20.171875000 +0200
@@ -1040,7 +1040,7 @@
     }
 
     if (map.generator == 1) {
-      make_random_hmap(MAX(1, 1 + SQSIZE 
+      make_random_hmap(MAX(1, 1 + get_sqsize() 
                           - (map.startpos ? game.nplayers / 4 : 0)));
     }
 
diff -Nurd -X.diff_ignore freeciv/server/generator/mapgen_topology.c 
freeciv/server/generator/mapgen_topology.c
--- freeciv/server/generator/mapgen_topology.c  2004-11-20 18:41:02.296875000 
+0200
+++ freeciv/server/generator/mapgen_topology.c  2004-11-20 19:14:16.609375000 
+0200
@@ -272,11 +272,12 @@
    * exept if separate poles is set
    */
   if (!topo_has_flag(TF_WRAPX) || !topo_has_flag(TF_WRAPY)) {
+    int sqsize = get_sqsize();
     if (map.separatepoles) {
       /* with separatepoles option strip poles are useless */
       ice_base_colatitude =
          (MAX(0, 100 * COLD_LEVEL / 3 - 1 *  MAX_COLATITUDE) 
-          + 1 *  MAX_COLATITUDE * SQSIZE) / (100 * SQSIZE);
+          + 1 *  MAX_COLATITUDE * sqsize) / (100 * sqsize);
       /* correction for single pole 
        * TODO uncomment it when generator 5 was well tuned 
        *      sometime it can put too many land near pole 
@@ -290,9 +291,18 @@
       /* any way strip poles are not so playable has isle poles */
       ice_base_colatitude =
          (MAX(0, 100 * COLD_LEVEL / 3 - 2 *  MAX_COLATITUDE) 
-          + 2 *  MAX_COLATITUDE * SQSIZE) / (100 * SQSIZE);
-    }  
-  }    
+          + 2 *  MAX_COLATITUDE * sqsize) / (100 * sqsize);
+    }
+  }
  
   map_init_topology(TRUE);
 }
+
+/*************************************************************************** 
+  An estimate of the linear (1-dimensional) size of the map.
+***************************************************************************/
+int get_sqsize(void)
+{
+  int sqsize = sqrt(map.xsize * map.ysize / 1000); \
+  return MAX(1, sqsize);
+}
diff -Nurd -X.diff_ignore freeciv/server/generator/mapgen_topology.h 
freeciv/server/generator/mapgen_topology.h
--- freeciv/server/generator/mapgen_topology.h  2004-11-20 18:41:02.343750000 
+0200
+++ freeciv/server/generator/mapgen_topology.h  2004-11-20 19:05:17.390625000 
+0200
@@ -18,11 +18,10 @@
 
 #define MAX_COLATITUDE 1000
 
-/* An estimate of the linear (1-dimensional) size of the map. */
-#define SQSIZE MAX(1, sqrt(map.xsize * map.ysize / 1000))
+int get_sqsize(void);
 
 /* size safe Unit of colatitude */ 
-#define L_UNIT (MAX_COLATITUDE / (30 * SQSIZE) )
+#define L_UNIT (MAX_COLATITUDE / (30 * get_sqsize()) )
 
 /* define the 5 region of a Earth like map 
    =========================================================