Complete.Org: Mailing Lists: Archives: freeciv-dev: September 2001:
[Freeciv-Dev] Re: [PATCH] map_adjust_* patch
Home

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

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: rf13@xxxxxxxxxxxxxxxxxxxxxx
Cc: jdorje@xxxxxxxxxxxxxxxxxxxxx, freeciv-dev@xxxxxxxxxxx
Subject: [Freeciv-Dev] Re: [PATCH] map_adjust_* patch
From: Gaute B Strokkenes <gs234@xxxxxxxxx>
Date: Sun, 02 Sep 2001 18:02:50 +0200

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.

> 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.

-- 
Big Gaute                               http://www.srcf.ucam.org/~gs234/
I just got my PRINCE bumper sticker..
 But now I can't remember WHO he is...


[Prev in Thread] Current Thread [Next in Thread]