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

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

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


Arnstein Lindgard wrote:
 > <URL: http://rt.freeciv.org/Ticket/Display.html?id=7279 >
 >
 > On Fri, 23 Jan 2004 09:05:09 -0800 rwetmore@xxxxxxxxxxxx wrote:
[...]
 > If you believe this inline has merely hidden a symptom, you should
 > compare against the previous code
[...]

The previous code does not invoke the bug you found with your macro
or Jason found when he tried it. Thus I'm sure you must agree the
previous code isn't particularly relevant, while the macro forms
that do show the bug are the ones that tell yoiu something.


Clearly Jason stated he tried adding local variable caching to the
macro and found a problem.

There is no question that at this point, he made the problem go away
by using inline. In fact he vigourously claims that macros are broken
and inline is *required* to fix this sort of problem.

This has clearly been shown to be a poor understanding of either
the problem uncovered or why the coding techniques make it go away.
His conclusions about the need for and usefulness of the inline
solution are also clearly wrong, as is happening a lot lately.


Fact: there is there is a poor coding practice in the hmap() usage that
is being hidden again by use of the inline function. There is a new
patch that is going to propagate this practice as hnat(). It needs to
be fixed properly and documented to prevent this from continuing.


More importantly though is that the fix to the previous code has been
done improperly here by use of inline over the more effective macro form.
Since performance is the reason for the change then choosing a solution
that has worse performance than the original macro under random
circumstances and even when it works is at best only as good as the macro
solutions you and Jason tried. One should chose the solution that
works all the time everywhere without portabilty and other issues.


The fix to map_to_index() clearly needs to be revisited and at least
reverted to the original macro form until both the performance and
other poor coding practice issue are solved.

[...]
 > Arnstein

Cheers,
RossW
=====