[Freeciv-Dev] Re: [PATCH] Corecleanup_07Part2 has been put in incoming

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Sep 02, 2001 at 11:43:32PM +0200, Gaute B Strokkenes wrote:
> On Sun, 2 Sep 2001, hawk@xxxxxxxxxxxxxxxxxxxxxxx wrote:
> > On Sun, Sep 02, 2001 at 07:58:15PM +0200, Gaute B Strokkenes wrote:
> >> On Sun, 02 Sep 2001, gs234@xxxxxxxxx wrote:
> >> 
> >> Allow me to elaborate.  The above would be perfectly fine if we
> >> inlined both mapstep() and normalize_map_pos().  As long as we
> >> don't, the perfomance loss associated with tacking two function
> >> calls on to half of the leaf functions in Freeciv is IMNSHO not
> >> justifiable on the gounds that you can reduce the number of places
> >> with topological knowledge in map.[ch] from ten to five.
> > 
> > In the long term I think we should/will have inline methods. I see
> > no reason to not make the critical methods inline.
> 
> The concern that was raised the last time this came up was that inline
> is not, strictly speaking, ISO C89 and also that current versions of
> GCC expand inline functions at a fairly late stage in the compilation
> process, so that optimization opportunities may be missed.
> 
> My own intuition is that just about all half-reasonable C compilers
> support inline functions, and that we can live with real function
> calls on the platforms that do not.  Also, GCC 3.0 and later has a
> better inliner, and the penalty is not likely to be high.
> 
> I suggest that someone with the spare cycles tries to run a test game,
> once with Jason's patch + inline normalize_map_pos() and once with no
> changes at all.

work.clean == cvs
work.adjust == map_adjust_[xy] as inline method
work.get_city == map_get_city as inline method
work.normalize == normalize_map_pos as inline method

$ size work.*/server/civserver
   text    data     bss     dec     hex filename
 562607   14736  607200 1184543  12131f work.adjust/server/civserver
 563935   14736  607200 1185871  12184f work.clean/server/civserver
 573407   14736  607200 1195343  123d4f work.get_city/server/civserver
 569295   14736  607200 1191231  122d3f work.normalize/server/civserver
 564031   14768  607200 1185999  1218cf work.sno/server/civserver

Normal compile flags plus "-Winline" was used.

Timing results were obtained by:
-------------
#!/bin/sh

cat >profile.rc <<EOF
set randseed 42       # repeat a particular game (random) sequence
set seed 42           # repeat a particular map generation sequence
set timeout -1          # run a server only autogame
set aifill 7            # fill to 7 players
set endyear 500
hard                    # make the AI do complex things
create Caesar           # first player (with known name) created and
                        # toggled to AI mode
start
EOF

for i in a b c d e f g
do
    time ./ser -r profile.rc >out.run_$i
done 2>&1 |
grep user >user_times
-------------

Results:

work.clean == cvs
user    1m15.020s
user    1m15.380s
user    1m15.330s
user    1m15.430s
user    1m16.800s
user    1m17.000s
user    1m16.660s

work.adjust == map_adjust_[xy] as inline method
user    1m15.100s
user    1m14.900s
user    1m15.230s
user    1m15.700s
user    1m15.370s
user    1m15.500s
user    1m15.480s

work.get_city == map_get_city as inline method
user    1m15.680s
user    1m15.900s
user    1m15.980s
user    1m16.070s
user    1m16.110s
user    1m16.160s
user    1m16.170s

work.normalize == normalize_map_pos as inline method
user    1m14.560s
user    1m14.750s
user    1m14.410s
user    1m14.350s
user    1m14.590s
user    1m15.040s
user    1m16.060s
 
You can draw conclusions. I'm too tired.

> If the difference in performance is not significant, and I do not
> think it will be, then we can change MAPSTEP and the others to be
> inline functions.  Until then, I suggest just leaving it as is.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Like the ad says, at 300 dpi you can tell she's wearing a
  swimsuit. At 600 dpi you can tell it's wet. At 1200 dpi you
  can tell it's painted on. I suppose at 2400 dpi you can tell
  if the paint is giving her a rash."
    -- Joshua R. Poulson

work.adjust.diff
Description: Text document

work.get_city.diff
Description: Text document

work.normalize.diff
Description: Text document