How to cleanup scripting?

[banduri]

Hi all

Currently I'm removing some parts in the buildings.txt which I think are ineffective. For instance If I look at the definition of BLD_SHIPYARD_BASE, BLD_SHIPYARD_ORBITAL_DRYDOCK and BLD_ART_PLANET I see this:

Code: Select all

BuildingType
    name = "BLD_SHIPYARD_BASE"
[...]
    location = And [
        Planet
        TargetPopulation low = 1
        Not Contains Building name = "BLD_SHIPYARD_BASE"
        OwnedBy empire = Source.Owner
    ]

wouldn't it be easier to write something that just checks for a targetpopulation (assuming only planets can have that attribute):

Code: Select all

    location = And [
        TargetPopulation low = 1
        Not Contains Building name = "BLD_SHIPYARD_BASE"
        OwnedBy empire = Source.Owner
    ]

The second question is about dependencies.

Code: Select all

BuildingType
    name = "BLD_SHIPYARD_ORBITAL_DRYDOCK"
[...]
    location = And [
        Planet
        Not Contains Building name = "BLD_SHIPYARD_ORBITAL_DRYDOCK"
        Contains And [
            Building name = "BLD_SHIPYARD_BASE"
            OwnedBy empire = Source.Owner
        ]
        OwnedBy empire = Source.Owner
    ]

Assuming the first one doesn't change any game mechanics couldn't I reduce it to:

Code: Select all

    location = And [
        Not Contains Building name = "BLD_SHIPYARD_ORBITAL_DRYDOCK"
        Contains Building name = "BLD_SHIPYARD_BASE"
        OwnedBy empire = Source.Owner
    ]

Here I do assume, that only planets can have a basic shipyard and that if the planet is owned by the empire the shipyard is to? (edit: also i think Source is the same in both cases and hence redundant )
An other question I have in mind is more about speed i guess.

Code: Select all

BuildingType
    name = "BLD_ART_PLANET"
[...]
    location = And [
        Planet
        Not Contains Building name = "BLD_ART_PLANET"
        Not Contains Building name = "BLD_ART_FACTORY_PLANET"
        Not Contains Building name = "BLD_ART_PARADISE_PLANET"
        OwnedBy empire = Source.Owner
        Planet type = [Asteroids GasGiant]
        Population high = 0
    ]

Is it right to assume that if the planet type filter is called first it would speed up some things (also I'm not sure how to measure it). I guess my question for the backend is: when the building.txt is evaluated, is it applied to the single planet the player clicked on, or are all objects in the universe evaluated and the result is applied to the objects,at the beginning of the turn? If the first one is true i think all checks for planets can be avoided while the later one would benefit from moving the planet type filter to the top. The last question is about

Code: Select all

location = Planet

. Currently I'm not aware of any in game possibility to build a building on anything else then a planet. Does the backend states that explicit? Would removing any check for 'location = planet' affect the game mechanics somehow?

[Geoff the Medio]

Is it right to assume that if the planet type filter is called first it would speed up some things (also I'm not sure how to measure it).

In general, yes, when filtering the whole universe (such as for an effect scope). If you filter first by a simple condition like Planet, it will use a cached subset of objects that only includes planets. If you omit that, depending what condition is next, it might have to check every object in the universe (unless the next condition also can use one of the cached subsets) instead of every planet in the universe.

I guess my question for the backend is: when the building.txt is evaluated, is it applied to the single planet the player clicked on...

You'll need to be more specific. Conditions are applied lots of times and places, and to different starting sets of potential matches depending what is being checked.

The last question is about

Code: Select all

location = Planet

. Currently I'm not aware of any in game possibility to build a building on anything else then a planet. Does the backend states that explicit? Would removing any check for 'location = planet' affect the game mechanics somehow?

Probably buildings can only be produced on planets, and probably this extra condition makes no or effectively no difference to any game behaviour or speed, but it was added intentionally, and doesn't hurt. In future, it might be possible to produce stuff in different places, and its easier to put such a filter everywhere rather than worry about when it is or isn't needed, particularly when scripts are using macros that might be included in different contexts, and it is occasionally helpful to have such a filter, so is a good habit to have.

[banduri]

Is it right to assume that if the planet type filter is called first it would speed up some things (also I'm not sure how to measure it).

In general, yes, when filtering the whole universe (such as for an effect scope). If you filter first by a simple condition like Planet, it will use a cached subset of objects that only includes planets. If you omit that, depending what condition is next, it might have to check every object in the universe (unless the next condition also can use one of the cached subsets) instead of every planet in the universe.

What are the cached sets beside planets? Thanks btw

[Geoff the Medio]

What are the cached sets beside planets?

Look at the GetDefaultInitialCandidateObjects functions in Condition.cpp.

[banduri]

Just to give an update what I tested the last few days.

My focus was to speed things up. For that I started a game with 1000 stars, many planets, 8 effect threads, one AI and let it run for 240 turns. I looked at the Server processing in bucket-01.txt to get a feeling for the numbers. At about turn 200 turns things started to get interesting since the processing slowly went from 3sec (turn 4) to 10sec (turn 200) to about 16secs at turn 240. I don't think this is comparable to games with ~200Stars but I assume it would be easier to spot the effect of any change I do.Changing/refineing location in buildings.txt, ship_parts.txt and ship_hulls.txt doesn't change the processing in a relevant manner. I hooked up valgrind to freeoriond to find a starting place for digging . The operater() methode from StoreTargetsAndCausesOfEffectsGroupsWorkItem (https://github.com/freeorion/freeorion/ ... .cpp#L1731) poped up but I didn't want to touch the Activation condition code. Turns out, there isn't much to do from the scripting point of view. Changing the order in nested "AND"s doesn't has a significant effect as far as i can tell. Since there are lots of "OwnedBy empire" statements i do assume a cached set for each empire could be usefull, but the callgraph of valgrind didn't point to it. Anything else i could try to decrease the server processing time? I simply don't want to wait that long between to Turns in late game.

[Geoff the Medio]

Since there are lots of "OwnedBy empire" statements i do assume a cached set for each empire could be usefull, but the callgraph of valgrind didn't point to it.

That could help if OwnedBy was the first condition being applied to a full universe of objects, but that's rarely the case. If there is another condition applied first, then the subsequent ones are applied to the matches of the earlier ones, rather than (if available) looking up the cached results.

StoreTargetsAndCausesOfEffectsGroupsWorkItem is a very general use function, and the time it takes to execute will depend on what is being evaluated, meaning what condition and on what starting set of objects. It would be more useful if you could identify a particular condition or combination of conditions that contributes substantially to the turn processing time, which could then be targeted for optimization, or perhaps the content for which could be reworked.

[banduri]

Since there are lots of "OwnedBy empire" statements i do assume a cached set for each empire could be usefull, but the callgraph of valgrind didn't point to it.

That could help if OwnedBy was the first condition being applied to a full universe of objects, but that's rarely the case.

That's what i thought.

StoreTargetsAndCausesOfEffectsGroupsWorkItem is a very general use function, and the time it takes to execute will depend on what is being evaluated, meaning what condition and on what starting set of objects. It would be more useful if you could identify a particular condition or combination of conditions that contributes substantially to the turn processing time

That's the sad part. I'm not able to profile the c++ part any further since i didn't find an easy way to do lineprofileing in cpp of single functions. gprof, gcc -pg -g, valgrind and kcachegrind are nice tools to get an understanding of what is happening. I'm not skilled in cpp . I'm able to add a timer or counter somewhere, writing some more debug output is also easy, but I'm lost if adding "DebugLogger() << m_effetcs_groups->Dump()" to "StoreTargetsAndCausesOfEffectsGroupsWorkItem::operator ( )" generates

Code: Select all

"terminate called after throwing an instance of 'std::length_error'
  what():  basic_string::_M_create

.
From a personal point of view I'm not a person who enjoys writing cpp much, but from time to time I have to give it a try

[Geoff the Medio]

Much of the debug logging info you probably would want is already available by enabling verbose logging in the UI options page, then starting / loading a game and playing a turn. Timing info for various effects execution steps will be output, as well as before/after effect execution gamestate.

[Geoff the Medio]

...It would be more useful if you could identify a particular condition or combination of conditions that contributes substantially to the turn processing time...

I implemented some (hopefully) optimizations to meter-setting effects, and removed / fixed some error messages being logged about ship part meter modifications being applied to ships that don't have that meter. I might have helped with effect processing times; testing reports about later-game turn times would be appreciated.