[Freeciv-Dev] Re: inlining functions

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Aug 20, 2001 at 11:05:36PM +0300, Markus Linnala wrote:
> Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx> writes:
> 
> > This is all nice. I hope however that we can beat most of the things
> > by smarter planning of data structures/data handling and not macros
> > and inlining. So it like your spotting of get_player and also the
> > ongoing discussion about the map normalization. All this will just
> > elimate the calls and not make them faster.
> 
> If you want to profile them, you can with my proposal. Yes, but
> you reall can't make these kind of functions lot faster.

No. But we can decrease the number of calls to these methods.

> I once made patch that tried to cache map_get_tile and
> calculation. Well, stupid idea. But I noticed, you could
> sometimes call map_get_tile with same arguments over 100 times
> in a row. Why? There is big room for improvement.
> 
> And yes you can optimize map_get_tile, by makeing it like this:
> ------------------------------------------------------------------------
> struct tile *map_get_tile_opt(int x, int y)
> {
>   assert(y>=0 && y<map.ysize);
>   assert(x>=0 && x<map.xsize);
>   return map.tiles+map_adjust_x(x)+y*map.xsize;
> }
> ------------------------------------------------------------------------
> 
> And make your iterators smarter. It is more effective to iterate:
> ------------------------------------------------------------------------
> int min_y = MAX(0, y);
> int max_y = MIN(map.ysize, y);
> int min_x = MAX(0, x);
> int max_x = MIN(map.xsize, x);
> for (y = min_y; y <= max_y; y++) {
>   for (x = min_x; y <= max_x; x++) {
>     struct tile *ptile = map_get_tile_opt(x, y);
>   }
> }
> ------------------------------------------------------------------------
> 
> And at many places this is situation. First we check that we
> have correct area, then we get tile and make calculations. So we
> check/normalize x and y twice.

I hope to get rid of this in the long term.

> We iterate over whole map, but only make something when we are
> at same continent.

I noticed this before and I also used it in my code sometimes.

> Continents are numbered. Make array size of continents. Save
> tiles at continent to the array. Iterate over them. Tiles can
> for a continent only finite (known) places. We update this array
> accordingly. If map has several islands, this usually saves 
> 
> Old:
> map_get_tile called max.xsize * map.ysize times
> 
> New:
> map_get_tile called: (average size of island)

Extra memory: xsize*ysize*landmass*sizeof(struct tile
*)+xsize*ysize*2*sizeof(short int) = 80*50*0.3*4+80*50*2*2 = 20800
bytes; this isn't too big

> We have have new and improved:
> map_get_tile called: 0 times before if.
> 
> We should also split big functions apart. ai_manage_explorer has
> 4 different, distinct parts. But now we can't find out what part
> makes biggest impact in general, ie. what part we should
> especially optimize. Another good example is
> find_something_to_kill 1463-1151, only 312 lines.
> ai_manage_explorer has big impact when you play game like 30
> players 15 explorers each. find_something_to_kill is always big
> at late part of game. Both use about 3-10% of server processing
> power.

I agree.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx