[Freeciv-Dev] Re: Profiling Civserver again

[Thue <thue@xxxxxxx>]

On Wednesday 25 July 2001 08:38, Jason Dorje Short wrote:
> Trent Piepho wrote:
> > On Tue, 24 Jul 2001, Ross W. Wetmore wrote:
> > > 2)  TRUE, map and map_adjust_x() will need to be defined in all
> > > functions that call the macro, whereas they weren't for the
> > > function call.
> >
> > Of course the function prototype for normalize_map_pos() is in the
> > same header file that defines map_adjust_x(), so that shouldn't be
> > a problem.
> >
> > But really, you're just covering up the real inefficiency.  It
> > would be more productive to figure out why map coordinates get
> > normalized so many times, and reduce that number.
>
> I draw your attention to the following lines from the gprof run:
>
> [12]    20.4  100.19  102.49  159008         really_generate_warmap
> [12] 38.41    0.00 797302800/948124931     normalize_map_pos [32]
>
> This says that of the 950 million times normalize_map_pos was called
> 800 million of them were in really_generate_warmap.  About 40% as
> much time was spent in normalize_map_pos as was spent in
> really_generate_warmap() itself.
>
> In other words, the following line accounts for about 4% of all
> Freeciv server CPU usage:
>       http://www.freeciv.org/lxr/source/server/gotohand.c?v=cvs#L313
>
> Aside from being astounding, this is totally unnecessary and can
> easily be improved on.
>
> With the attached patch, the use of normalize_map_pos is dropped and
> the normalization is handled manually.  This could be improved on
> much more - the normalization is still done for each of the 8
> adjacent spots - but it is more efficient than the function call or
> even a standard macro would be.
>
> jason

I would prefer if you made a adjc_iterate_dir(center_x, center_y, 
itr_x, itr_y, itr_dir) macro. Then we could use it other places too, 
without having hand-optimizations spread all over the code.
Actually I would prefer to put the optimization in a macro beside the 
other macros even if we only use it this once.
As Jarda Benkovsky said, not having optimizations spread around will 
allow us to change the map topology. Fx it is relatively easy to add 
support for a flat worldmap.

Oh, and I think it has been established that you don't want to use 
modulo :). As in the patch to normalize_map_pos() you want to use a 
conditional instead. Besides, all the tile will be adjacent to real 
ones, so you don't need a "while (x >= map.xsize)" like in 
normalize_map_pos(), just an "if (x >= map.xsize)".

-Thue