[Freeciv-Dev] Re: [RFC] square_dxy_iterate

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sat, Feb 23, 2002 at 07:23:27AM -0500, Jason Short wrote:
> Raimar Falke wrote:
> > On Fri, Feb 22, 2002 at 06:04:45PM -0500, Jason Short wrote:
> > 
> >>Raimar Falke wrote:
> >>
> >>>>From the profile of an autogame (with disabled CHECK_MAP_POS):
> >>>Flat profile:
> >>>
> >>>
> >>>80695222/2379222=33.916 calls of normalize_map_pos for each call of
> >>>invasion_funct. It turns out that square_dxy_iterate is the
> >>>problem. And the second one (is_terrain_near_tile) uses adjc_iterate.
> >>>
> >>>Are there any is_border patches or similar handy which solve this?
> >>>
> >>Ross's corecleanups (kindof) extend the IS_BORDER_POS check to check for 
> >>arbitrary (map_dist) distances from the border.  Under the current 
> >>topology this is easy; under others it will be less so (but still 
> >>doable, plus our definition that IS_BORDER_POS(..)=>FALSE means the 
> >>position may or may not be on the border gives us a good default 
> >>option).  I'll put together a patch sometime this weekend, I suppose - 
> >>it should be quite short.
> >>
> > 
> > Ok.
> 
> See attached patch.
> 
> I've changed IS_BORDER_MAP_POS to take a distance argument as well. 
> This is similar to what Ross's changes do (it uses a more robust check 
> for IS_BORDER than is currently used), except I've unified it into one 
> macro call, rather than have a
> 
> #define IS_BORDER_MAP_POS(x, y) _IS_BORDER_MAP_POS(x, y, 1)
> 
> I've also added IS_BORDER checks to all of the iterator macros (why 
> not?).  I've taken the liberty of rewriting adjc_iterate in terms of 
> square_iterate (which should give identical behavior, aside from the 
> addition of the IS_BORDER check).
> 
> Since (just about) all of the iterators use a different internal 
> notation, the changes may look a bit inconsistent.
> 
> In limited testing, this change caused about a 3% increase in server 
> time efficiency (i.e. 3% decrease in running time [1]).  This is less 
> than I expected (normalize_map_pos accounts for 4.5% of server runtime 
> according to the profile, and that doesn't include all of the 
> function-call overhead which will also be included in the calling 
> function's time) - which probably means there are a lot of normalize 
> calls left.

Flat profile:

Each sample counts as 0.01 seconds.
  %   cumulative   self              self     total           
 time   seconds   seconds    calls  ms/call  ms/call  name    
 11.59     93.36    93.36   181955     0.51     0.71  really_generate_warmap
  3.15    118.71    25.35 564336118     0.00     0.00  unit_type_flag
  2.64    139.96    21.25 332564683     0.00     0.00  tech_exists
  2.62    161.07    21.11 400650742     0.00     0.00  map_get_terrain
  2.40    180.38    19.31 365966093     0.00     0.00  normalize_map_pos
  2.17    197.83    17.45 167296801     0.00     0.00  base_city_map_to_map
  1.96    213.63    15.80 522552696     0.00     0.00  map_get_tile
  1.82    228.33    14.70 332000574     0.00     0.00  get_invention
  1.79    242.76    14.43  7674804     0.00     0.00  win_chance
  1.79    257.15    14.39 522630554     0.00     0.00  contains_special
  1.69    270.78    13.63 219704496     0.00     0.00  find_genlist_position
  1.65    284.10    13.32 215526217     0.00     0.00  map_get_city
  1.54    296.54    12.44 407757155     0.00     0.00  unit_type
  1.42    307.95    11.41 120089132     0.00     0.00  get_from_mapqueue
  1.31    318.50    10.55 271197229     0.00     0.00  city_owner
  1.28    328.83    10.33 257215016     0.00     0.00  is_valid_city_coords
  1.22    338.68     9.85 119849806     0.00     0.00  add_to_mapqueue
  1.21    348.44     9.76  4509328     0.00     0.00  road_bonus
  1.19    358.06     9.62 485224379     0.00     0.00  get_unit_type
  1.19    367.68     9.62    64697     0.15     3.43  find_something_to_kill
  1.14    376.89     9.21  2795579     0.00     0.01  num_known_tech_with_flag
  1.12    385.88     8.99 27670061     0.00     0.00  internal_lookup
  1.05    394.36     8.48 18393335     0.00     0.00  base_city_get_trade_tile
  1.05    402.80     8.44 190521531     0.00     0.00  improvement_exists
  1.04    411.15     8.35  8756109     0.00     0.00  dangerfunct
  1.02    419.37     8.22 219696205     0.00     0.00  genlist_iterator_init
  1.00    427.42     8.05 21743168     0.00     0.00  base_city_get_food_tile
  0.97    435.20     7.78 133984066     0.00     0.00  city_got_building


                0.22    0.00 4818024/564336118     find_boat [156]
                0.22    0.00 4920342/564336118     ai_choose_defender_limited 
[214]
                0.23    0.00 5210128/564336118     find_something_to_kill [7]
                0.30    0.00 6621971/564336118     process_attacker_want [190]
                0.33    0.00 7452343/564336118     do_unit_goto [31]
                0.37    0.00 8331336/564336118     ai_choose_attacker [192]
                0.64    0.00 14261484/564336118     get_defense_rating [75]
                0.81    0.00 18097812/564336118     assess_danger [28]
                1.04    0.00 23049458/564336118     unit_win_chance [40]
                1.16    0.00 25823802/564336118     
unit_really_ignores_citywalls [115]
                1.60    0.00 35528301/564336118     
military_advisor_choose_build [8]
                2.61    0.00 57997292/564336118     can_build_unit [30]
                4.74    0.00 105428106/564336118     can_build_unit_direct [41]
                9.35    0.00 208175638/564336118     ai_choose_defender_versus 
[20]
[58]     3.1   25.35    0.00 564336118         unit_type_flag [58]
-----------------------------------------------

Where in ai_choose_defender_versus() is unit_type_flag() called? Is my
gcc/gprof faulty?

                0.00    0.00    3194/332564683     precalc_tech_data_helper 
[785]
                0.00    0.00   25988/332564683     update_research [343]
                0.03    0.00  525441/332564683     build_required_techs_helper 
[412]
               21.21    0.00 332000574/332564683     get_invention [38]
[66]     2.6   21.25    0.00 332564683         tech_exists [66]
-----------------------------------------------

enum tech_state get_invention(struct player *pplayer, Tech_Type_id tech)
{
  if (!tech_exists(tech)) {
    return TECH_UNKNOWN;
  }
  return pplayer->research.inventions[tech].state;
}

should be replaced with

enum tech_state get_invention(struct player *pplayer, Tech_Type_id tech)
{
  assert(tech >= 0 && tech <game.num_tech_types);
  return pplayer->research.inventions[tech].state;
}

                0.87    0.00 16397822/365966093     ai_manage_explorer [51]
                1.02    0.00 19347751/365966093     
city_exists_within_city_radius [137]
                1.05    0.00 19860225/365966093     evaluate_improvements [18]
                1.30    0.00 24556266/365966093     server_set_tile_city <cycle 
2> [96]
                1.38    0.00 26223192/365966093     really_generate_warmap [13]
                2.84    0.00 53845473/365966093     is_real_tile [154]
                3.09    0.00 58621264/365966093     road_bonus [71]
                3.92    0.00 74268199/365966093     city_map_to_map [73]
[70]     2.4   19.31    0.00 365966093         normalize_map_pos [70]
-----------------------------------------------

                0.00    0.00   45360/53845473     tile_move_cost_ai [665]
                0.01    0.01  195111/53845473     check_units [338]
                0.04    0.07 1310964/53845473     base_map_to_city_map [163]
                1.68    2.76 52293996/53845473     map_get_player_tile [108]
[154]    0.6    1.73    2.84 53845473         is_real_tile [154]
-----------------------------------------------

  if (!is_real_tile(x, y)) {
    freelog(LOG_ERROR, "Trying to get nonexistant tile at %i,%i", x, y);
  }
  nearest_real_pos(&x, &y);

should be removed since map_inx has a CHECK_MAP_POS check.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 checking for the vaidity of the Maxwell laws on this machine... ok
 checking if e=mc^2... ok
 checking if we can safely swap on /dev/fd0... yes
    -- kvirc 2.0.0's configure