[Freeciv-Dev] Re: [PATCH] map_adjust_* patch

[Gaute B Strokkenes <gs234@xxxxxxxxx>]

On Fri, 31 Aug 2001, jshort@xxxxxxxxxxxxx wrote:

> jasonIndex: common/city.h
> ===================================================================
> RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
> retrieving revision 1.86
> diff -u -r1.86 city.h
> --- common/city.h     2001/08/26 13:10:12     1.86
> +++ common/city.h     2001/08/31 06:03:14
> @@ -130,10 +130,10 @@
>      for (MCMI_y=0; MCMI_y<CITY_MAP_SIZE; MCMI_y++) { \
>        if (! ((MCMI_x == 0 || MCMI_x == (CITY_MAP_SIZE-1)) \
>            && (MCMI_y == 0 || MCMI_y == (CITY_MAP_SIZE-1))) ) { \
> +     x_itr = city_x + MCMI_x - CITY_MAP_SIZE/2; \
>          y_itr = city_y + MCMI_y - CITY_MAP_SIZE/2; \
> -        if (y_itr < 0 || y_itr >= map.ysize) \
> -       continue; \
> -     x_itr = map_adjust_x(city_x + MCMI_x - CITY_MAP_SIZE/2);
> +        if (!normalize_map_pos(&x_itr, &y_itr)) \
> +       continue;
>  
>  #define map_city_radius_iterate_end \
>        } \

I'm a bit ambivalent about using normalize_map_pos() inside iteration
macros like this one, and in low-level utility functions like
map_get_tile().  While it knocks the correctness issue out cold, I
worry about the overhead associated with a function call and
subsequent compiler optimizer confusion in these spots which are
executed an ungodly number of times during each ai turn.  IIRC
normalize_map_pos(), map_get_tile() and a few others are responsible
for about 20% of the CPU usage when running an average all-ai game,
which is already ridiculously much.  I think it's perfectly acceptable
for the iteration macros at least to know what topologies look like.

If you profile a standard ai game both before and after applying your
patch and find no noticable slowdown, none of the above applies.  8-)

Also it would be nice if you could convert the places that you touched
to use map_inx() where appropriate.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
Hello.  I know the divorce rate among unmarried Catholic
 Alaskan females!!