[Freeciv-Dev] Re: Profiling Civserver again

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

Absolutely correct! 

Will make sure the first % is left in and the code is properly 
commented on any patches so this doesn't happen again.

Cheers,
RossW

At 07:43 PM 01/08/01 +0200, Lino Mastrodomenico wrote:
>Ross W. Wetmore wrote:
>> At 05:21 AM 01/08/01 +0300, Gregory Berkolaiko wrote:
>> >Jason is right when he says that cleaning up map_adjust_x should do good.
>> >I think a good portion of CPU time spent in get_map_tile is wasted on two
>> >modulus ( % ) operations in map_adjust_x.  One is completely unnecessary,
>> >as was mentioned earlier.  Is it possible to get rid of the second one(s)
>> >in a safe way without converting the macro to a function?
>[...]
>> If you do the same trick with map_adjust_x, you can test for the border
>> cases in one operation and avoid redundant "%"s. If you assume map.xsize
>> is always positive (would be an interesting game if it weren't), then you
>> can avoid one "%". If you assume the only valid range for out-of-bounds
>> is one map.xsize above or below, then you can drop the "%"s. But with the
>> added border check, I would not worry about the expense of saving 2 % vs
>> adding one "-" on 100 border ops in a 4000 map, since the % is actually
>> safer.
>>
>> #define map_adjust_x(X)                        \
>>   ((unsigned)(X) < (unsigned)map.xsize ? (X)   \
>>   : ((X) < 0 ? (X) % map.xsize + map.xsize : (X) % map.xsize))
>
>Warning: this change will reopen this bug: 
>
>  http://www.freeciv.org/cgi-bin/bugs?findid=749
>
>And please note that gcc with -O2 is perfectly capable to optimize trivial 
>cases like this: the module is performed only once.
>
>E.g.: for the x86 it generates something like this:
>
>***
>        movl    map, %ecx
>        movl    %edx, %eax
>        sarl    $31, %edx
>        idivl   %ecx
>        testl   %edx, %edx
>        jns     .L69
>        addl    %ecx, %edx
>.L69:
>***
>
>ciao
>
>-- 
>Lino Mastrodomenico
>E-mail: mastro@xxxxxxxxxx
>
>