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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Sep 02, 2001 at 06:02:50PM +0200, Gaute B Strokkenes wrote:
> On Sat, 1 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > 
> > unpatched:
> > user    0m12.720s
> ...
> > 
> > patched:
> > user    0m13.870s
> ...
> > 
> > 13.8/12.7=1.08661417323
> 
> ~8.7 percent?  That's too much.  Eight patches like that, and the
> runtime will double.
> 
> Now, I'm all for hiding yucky stuff like coordinate normalisation
> etc. in standard interfaces, but what one needs to realise is that
> this is some of the most performance sensitive code in Freeciv.  
> 
> The purpose of the _iterate macros is to allow user code to iterate
> efficiently over some region in a safe and efficient mannner with
> minimum fuss.  
> 
> The best thing would be if we went all the way and made
> normalize_map_pos() and inline function.  That would (hopefully)
> eliminate the performance impact, while eliminating local knowledge.
> Somebody just needs to do the infrastructure work (shouldn't be too
> hard; just call AC_C_LINE in configure and ask the Amiga people to do
> the equivalent thing) and do the profiling to confirm that it works.

Since this performance debate will not end sooner and I'm also for
inline and against macro I will make a patch.

> > I liked Ross's idea of a border position which would needs extra
> > checking. Jason can you do such a patch?
> 
> I think that this sort of checking is a good way to improve
> performance in cases like these, but if you replace the current case
> with a call to is_border_tile() or whatever and then a conditional
> call to normalize_map_pos() then you have given up on the idea that
> all the position cleanup gunk should be in one place.  So in effect
> you have not improved anything, but you have made Freeciv run slower.
> 
> >> Also it would be nice if you could convert the places that you
> >> touched to use map_inx() where appropriate.
> > 
> > Nice but not necessary.
> 
> What's your point?  I see no essential difference in replacing ad hoc
> code to perform the equivalent of normalize_map_pos() with
> normalize_map_pos() and replacing ad hoc code to perform the
> equivalent of map_inx(), except that there is no excessive performance
> penalty associated with the latter.

"not necessary" for patch application. But as you stated in the other
mail: we are talking about an empty set.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  Living on earth may be expensive, but it includes an annual free trip
  around the sun.