[Freeciv-Dev] Re: Profiles from huge game

[Ben Webb <ben@xxxxxxxxxxxxxxxxxxxxxx>]

On Sun, 2 Sep 2001, Sebastian Bauer wrote:

> > The client:
> >  time   seconds   seconds    calls  us/call  us/call  name
> >  30.79      2.79     2.79     1337  2086.76  3175.42  update_all_effects
> >   7.95      3.51     0.72  1595230     0.45     0.57
> fill_ranges_improv_lists
> >   6.07      4.06     0.55   320937     1.71     5.25
> fill_tile_sprite_array
> >   5.08      4.52     0.46  2957889     0.16     0.29  get_drawn_char
> >   4.30      4.91     0.39  3524832     0.11     0.11  map_get_terrain
> >   3.64      5.24     0.33   797205     0.41     0.99
> improvement_redundant
> >
> > Sebastian, can you put a hand on this?

        First, let me explain what these two functions
(update_all_effects and fill_ranges_improv_lists) do. They are part of my
generalised improvements patch, most of which Sebastian seems to have
applied to CVS. While this is, IMHO, a good thing, it really is a proof of
concept patch, and will definitely require plenty of optimisation and
tweaking before the next version. The "effects" refer to the "effect"
field in buildings.ruleset - they are conferred by constructing
buildings/improvements in your cities. For example, an effect might be
"double the luxury production in this city". The aim is to completely
replace the current hard-coded behaviour of buildings, to allow for more
flexible modpacks and projects such as FreecivAC[*] (Alpha Centauri
compatibility).

[*] Blatant plug: http://bellatrix.pcl.ox.ac.uk/~stewart/fcindex.html

        Even when you construct a building, it may be made redundant by
another building (for example, a Barracks has no effect if you have the
Academy). fill_ranges_improv_lists makes a list of improvements that can
affect a given city, which is then used to test if a building is redundant
in this way. (This is controlled by the "equiv_range", "equiv_dupl" and
"equiv_repl" fields in buildings.ruleset.) update_all_effects figures out
which buildings are redundant or obsolete, and then applies the effects of
"active" buildings. This is complicated by the fact that some effects are
conditional on other buildings or effects (e.g. cond_eff field) and so,
for simplicity, I presently do this for _all_ buildings in the game.

> I think much of  update_all_effects() and the other calls are really
> unneccessary

        This isn't strictly speaking true. Everywhere where
update_all_effects is currently being called, there needs to be some kind
of update of effects. However, the call can certainly be optimised. For
example, I suspect the main problem with the client is that all effects
for all cities are updated every time a city packet is received. It is
only necessary to update the effects if such a packet actually adds or
removes buildings, and even then shortcuts can be made if the resultant
effects are not too interdependent. Since update_all_effects calls
fill_ranges_improv_lists repeatedly, such an optimisation should radically
cut down the number of calls to this function too. I'm pretty busy (having
just returned from holiday) right now, but will attempt to get a suitable
patch out soon.

> but I wanted to wait for the next step of the generalizaton
> where time needed will further increases (this is the price).

        Building the lists of active effects (which is what
update_all_effects does) is the major part of the work, IMHO. Actually
using these effects is then a matter of using a simple "effect iterator" -
the only speed concern is that some of the city handling code needs to be
reordered, so that the number of separate iterations over the effect list
can be kept to a minimum.

> Ben wanted to create the patch but actually he is on holliday.

        Well, first I have to figure out how much of my original impr-gen
patch is actually now in CVS. Most of it seems to be there - what did you
miss out? I still see some instances of the dodgy B_LAST constant, which I
thought I'd killed for good in my patch, for example.

> Before the code is optimzied I really would like to have that this is
> really nearly free of bugs.

Nobody seems to have complained about segfaults or logic errors so far...

        Ben
-- 
ben@xxxxxxxxxxxxxxxxxxxxxx           http://bellatrix.pcl.ox.ac.uk/~ben/
"The dog did nothing in the night-time." "That was the curious
incident," remarked Sherlock Holmes.
        - 'Silver Blaze', Sir Arthur Conan Doyle