Complete.Org: Mailing Lists: Archives: freeciv-dev: June 2004:
[Freeciv-Dev] Re: (PR#8982) City.c cleanup
Home

[Freeciv-Dev] Re: (PR#8982) City.c cleanup

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: pim@xxxxxxxxxxxx
Subject: [Freeciv-Dev] Re: (PR#8982) City.c cleanup
From: "rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>
Date: Thu, 17 Jun 2004 13:34:31 -0700
Reply-to: rt@xxxxxxxxxxx

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



Per Inge Mathisen wrote:

> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8982 >
> 
> On Mon, 14 Jun 2004, Jason Short wrote:
> 
>>I'm not sure that changing arbitrary static functions to inline is a
>>good idea.

Changing things to inline was discussed, shown to be a bad thing with no 
real redeeming features beyond performance degradation and has thus been 
preferentially adopted as usual :-).

This longstanding Freeciv rule was there for a reason, especially for those 
that do not understand or take the time to understand reasons.

[...]
> Now, why does freeciv run slower with my patch? No, it does not have
> anything to do with inline, since it runs even slower without it... Does
> const give slower code??

Changing things to function calls adds enormous performance degradation, 
only partially and occasionally recovered by inlining since this is now an 
optional compiler element. If you want performance (no longer true it 
appears), then you need to use real "C" elements like macros that give 
guaranteed results. You also need to understand when some constructs are 
being treated as static or const and what the difference is in the code 
optimization context that you are destroying in ways similar to inlining.

> Numbers:
>  17.600s for cvs head
>  17.950s for my patch
>  18.070s for my patch minus inline
> 
> Version of the patch without inline attached. Apply whichever version you
> feel like.
> 
>   - Per

The original code seems to be the best choice. Patch doesn't look 
particularly useful :-).

Cheers,
RossW
=====





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