[Freeciv-Dev] [RFC][Patch] Inline

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

There was a major network breakdown (three universities and ten other
research institutes were offline). So I could commit patches. I took
the time to work in the performance front.

The attached patch is a first cut on inlining functions. It does:
 - move city and unit list to their own files: unit_list.[ch] and
 city_list.[ch]
 - demerge dependencies
 - add missing dependencies
 - move type declaration into new files improvement_types.h,
 tech_types.h and unittype_types.h. I don't know a way to avoid this.
 - move functions which are called a lot out of foo.c and into
 foo_inline.h. I just took the roughtly top 40 most called functions.
 - allow the inlining of these functions if USE_INLINE is
 defined. There is currently no support for this in configure or
 makefiles. You have to specify it by hand. Make sure you also have at
 least -O specified.
 - move some functions to other files where they really belong. Saves
 an include.
 - demacrofy some of the macros of map.h
 - inline some static functions.

TODO:
 - add missing includes to the client files
 - add some configure magic to discover the inline keyword the
 compiler recognized.
 - add a configure to enable/disable inlining
 - since this is a first cut and focused on the dependency problems
 and not so the performance increase I have just inlined the most
 called functions. AFAIK there is no formal criteria which can be used
 to determine if inling of this function is worth it. Only solution is
 to perform the inline and test the performance. This may lead on
 different CPU/platforms to different results.

<dream mode on>
There is an automatic patch tester. You give this bot a tree and a
list of patches. It will apply the patch, configure and build the code
(using options you specify). It will then run N autogames based on a
rc-template. It will measure the performance. This will be done for
every patch and the stock tree. Extra points for comparing the
savegames to the stock version. Extra points for measuring average and
variance. More extra points for feeding the results through
gnuplot. Extra points for fetching the stock tree from the CVS
repository.

So I would just put some patched into the patches/ directory before I
go to sleep and have a nice analysis the next morning. This will also
allow an analysis which functions are worth inlining (DIR_CW for
example isn't called in an autogame. Such automatic measurement will
also give us a feeling what gains can be expected by inlining certain
functions. I have currently no feeling for this.
<dream mode off>

Results:
 stock: 62.15s
 with this patch and no inlining: 80.97s (difference because of
 converting map_inx, dirstep,.. to functions)
 with this patch and inlining: 40.46s
 with this patch and inlining and NDEBUG: 36.83s
 with this patch and inlining and -O3: 39.48s

If these results can be beaten by macros I would like to get such a
patch to get a look at the asm code.

Start of profile of stock version:
  %   cumulative   self              self     total
 time   seconds   seconds    calls  ms/call  ms/call  name
 13.16      8.18     8.18    18815     0.43     0.61  really_generate_warmap
  3.72     10.49     2.31 41884248     0.00     0.00  unit_type_flag
  2.72     12.18     1.69 55186231     0.00     0.00  map_get_tile
  2.45     13.70     1.52 15637336     0.00     0.00  base_city_map_to_map
  2.40     15.19     1.49      212     7.03    11.38  check_fow
  2.27     16.60     1.41 47148666     0.00     0.00  contains_special
  2.20     17.97     1.37 29658031     0.00     0.00  map_get_terrain
  2.16     19.31     1.34 25275363     0.00     0.00  normalize_map_pos
  1.80     20.43     1.12 21514018     0.00     0.00  find_genlist_position
  1.58     21.41     0.98 22481272     0.00     0.00  is_valid_city_coords
  1.48     22.33     0.92     1142     0.81     2.65  ai_manage_explorer
  1.46     23.24     0.91 11092796     0.00     0.00  get_from_mapqueue
  1.38     24.10     0.86 21512617     0.00     0.00  genlist_iterator_init
  1.35     24.94     0.84 11070748     0.00     0.00  add_to_mapqueue
  1.30     25.75     0.81 15641268     0.00     0.00  map_get_city
  1.30     26.56     0.81  4259727     0.00     0.00  internal_lookup

Start of profile of "with this patch and inlining":
  %   cumulative   self              self     total
 time   seconds   seconds    calls  ms/call  ms/call  name
 23.75      9.61     9.61    18815     0.51     0.52  really_generate_warmap
  2.60     10.66     1.05      212     4.95     4.95  check_fow
  2.20     11.55     0.89  4259727     0.00     0.00  internal_lookup
  2.15     12.42     0.87   324187     0.00     0.00  
city_exists_within_city_radius
  2.10     13.27     0.85   445218     0.00     0.00  road_bonus
  2.00     14.08     0.81  1234214     0.00     0.00  base_city_get_trade_tile
  1.95     14.87     0.79      887     0.89     0.90  ai_select_tech
  1.88     15.63     0.76  1362567     0.00     0.00  base_city_get_food_tile
  1.83     16.37     0.74  3685593     0.00     0.00  city_affected_by_wonder
  1.80     17.10     0.73  1276495     0.00     0.00  amortize
  1.75     17.81     0.71  7183527     0.00     0.00  
can_player_build_unit_direct
  1.71     18.50     0.69   230604     0.00     0.01  city_desirability
  1.66     19.17     0.67     1142     0.59     1.54  ai_manage_explorer
  1.58     19.81     0.64   114947     0.01     0.02  ai_choose_defender_versus
  1.53     20.43     0.62     5346     0.12     0.83  evaluate_improvements

Usage:
 - unpack
 - patch -p1 <inline1.diff
 - make clean
 - either 
     make 
   or 
     make CPPFLAGS="-DUSE_INLINE"

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Reality? That's where the pizza delivery guy comes from!"

inline1.diff.bz2
Description: BZip2 compressed data