[Freeciv-Dev] Re: (PR#7279) Macro optimizations

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

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


Be very very very careful with this approach ...

Using a macro for normalize_map_pos() is the single most effective step
for improving performance in Freeciv. Removing this capability is not
something to be done without a major review of the options and reasons
for so doing. Even without performance, the extent and scope of usage
of this function makes it one to touch with utmost caution.

Note, it has always been understood that at some stage this function
would grow too complex for simple optimization tricks, so what you are
proposing is not forbidden, but I do not think you have sufficiently
prepared the RFC ground for making it happen now. You need to be
strongly prepared for this level of justification, and to expect a lot
of really hard questions during the process.

But there is room for some really neat feature growth if you get it right
so it is certainly worth spending the effort.


The major reason for using a virtual function over a parameter option
to a general function is that there is not a lot of common code in the
individual cases, thus having a lot of specially tailored functions
keeps things in their separate boxes.

I don't think this is the case. I suspect that the benefits of having
a common set of steps in a general function with a few instances where
special casing handles special issues is still more likely to be the
best solution here. Note this does not rule out having one of the
parameters be a virtual function/list to deal with these situations.
PathFinding explored this a bit if you need examples.

The latter is analogous to a delegate flavour as opposed to a subclass
approach. In this situation, it may still be possible to use a macro
for most topologies with only the more complex ones falling back to
virtual functions, thus limiting the performance impact of being forced
to use a function call to topologies that can't do it any other way.


In particular, you should allow for adding filters to existing topologies
such that one can do an elliptical map from FlatEarth by excluding corner
tiles. This is an addon check to the current code common across all
topologies if you want.

Another addition is some sort of inverse to filters which basically
creates specially handled tiles such as transfer portals. Again these
are addons to existing code.

As has been stated elsewhere, hexagonal topologies are fairly trivial
mods to the current set.

There are a number of other extensions that have been mulled over once
the current basic 8 are implemented fully. But hopefully you get some
feel for the sort of extensions you should be considering, at least to
the point of not building obstacles in their way.

Writing common code over and over again in individual methods is not
going to go over very well, so at the least you need to have a counter
case that avoids this structural nightmare scenario.


Cheers,
RossW
=====


Marcelo Burda wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >
> 
> Hi,
> I am working in Extending and Optimizing normalize_map_pos, see(#7287), 
> plz not touch it!, you can mod is_normal_map_pos if you like
> i make big change to the city_map_to_map() function family  and all code 
> using its ! plz not touch it too!
> 
> i use pointer to function to implement normalize_map_pos, this alow to 
> make optimized function to differents topologies, macro or inline 
> function are too limited.
> 
> this patch include some mapgen code too and a rewrited overview this is 
> a litle long sorry.
> 
> Marcelo