[Freeciv-Dev] Re: (PR#7350) Map radius change with city size

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7350 >


Jason Short wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7350 >
> 
>>[rwetmore@xxxxxxxxxxxx - Sat Apr 10 05:03:40 2004]:
> 
>>The advantage of doing it in general iteration is that usually a
>>closest tiles first order is going to find the optimum whatever
>>over an array order iteration. These effects are actually part
>>of the reason why the corecleanups consistently ran 50% faster
>>than comparable CVS. The fact that you forced the core DIR_DX
>>handling to be the same as the GUI, rather than using rotational
>>order in the core and array order only for GUI tiling is a similar
>>case to this more limited citymap example.
>>
>>Thus doing things in the order you argued for and chose by getting
>>in a partial patch back then, i.e. link the two, then migrate,
>>rather than keep the core and GUI separate and migrate just the
>>core leaving the GUI for a proper overhaul, has had signficant
>>and detrimental results. We are now at the point where the GUI
>>is being cleaned up with a separate set of coordinates, but the
>>unnecessary linkages and performance/robustness/simplicity
>>cleanups are still not done. And you are still fighting them now
>>with O(5) patches that still handle things from the array order
>>perspective.
> 
> 
> On the topic of the direction8 ordering...
> 
> enum direction8 is just a list of directions.  Nobody (not the GUI or
> the core code) needs this to be in any particular order so long as they
> use the correct interface (including DIR_REVERSE, dir_cw, etc.) for
> manipulating the directions.  Rotational ordering is faster for some
> operations, like dir_cw, which are currently used only by the GUI.  It's
> also more elegant, meaning fewer lines of code in these functions.

The value of rotational ordering was mainly in the core adjacent iterators
for which there was one common algorithm called with various arguments.
This is extremely fast and robust.

Only the GUI needed array ordering for tiling purposes, and thus needed
two flavours of iterators, the directional ones and those for tiled ops.
In fact directional ordering was introduced into the GUI by Thue, and
moved to the core as part of the cleanups to consolidate iterators as the
common code for doing such operations - one of Thue's pre-requirements for
the mapgen.c changes I had submitted.

> Changing the ordering meant fixing all of the users to use the
> interface.  The only reason you wanted to separate the GUI from the core
> ordering was because you didn't want to bother to clean up the GUI code.

Yes. There was no need to link the directional and tiled cases in the GUI
so there was a single form, at least until the GUI was cleaned up.

> AFAIK there's nothing preventing us from reordering the directions now.

That would be nice, but it implies the current code is rather inefficient
in handling shared edges where tiles overlap. Using array ordering, one
draws the top and left edge, then updates all tiles ignoring the north and
west sides which are done by the south and east edges of the preceding
rows and columns.

>  However if it's just a question of speed, I suspect the rotational
> ordering will end up making the server slower.

As usual your suspicions seem more to support your desired goal of the
moment than reality. As has been proven, there is a significant gain
in using directional ordering. The corecleanups ran 50% *faster* than
comparable CVS,and a big part of this was the speed of the iterators.
Those iterators all run as a single loop in which "break" works as well.
You should actually look at the code and its commentary some time.

> As an aside, nobody who actually played the corecleanups would claim
> that they were faster than CVS code.

You mustn't have played or clocked them then :-).

You should try running some autogame tests with both to see. For 18 months
I did this regularly as part of the testing before releasing the patches.
This removes all the subjectivity about what things seem to be. I know,
and if you want to keep making such claims, you should as well.

> jason

Cheers
RossW
=====