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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Sep 01, 2001 at 04:09:06AM +0200, Gaute B Strokkenes wrote:
> 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-)

No profile but overall run time:
script:
set randseed 42
set seed 42
set timeout -1
set aifill 7
set endyear -2000
hard
create Caesar
start

unpatched:
user    0m12.720s
user    0m12.750s
user    0m12.690s
user    0m12.770s
user    0m12.760s
user    0m12.500s
user    0m12.630s

patched:
user    0m13.870s
user    0m13.840s
user    0m13.810s
user    0m13.770s
user    0m13.740s
user    0m13.880s
user    0m13.820s

13.8/12.7=1.08661417323

I liked Ross's idea of a border position which would needs extra
checking. Jason can you do such a patch?

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

Nice but not necessary.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  The trick is to keep breathing.