LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
289 stars 160 forks source link

Technical Questions re: Performance & Memory #6081

Closed RecursiveStar closed 4 years ago

RecursiveStar commented 4 years ago

It's my understanding that:

Memory values and local variables have a negative impact on memory, and function calls have a negative impact on performance.

If a function call is expensive and a local variable can be used to cache the value, this is a performance optimization.

I have three questions for modders who are more familiar with the game and with C++ programming than I am:

1) Do integer and boolean memory values have a significant impact on memory? I've added new memory values to the diplomacy AI, and I know Civ V has limited memory. Obviously they have some impact, but if the total is relatively negligible and much more memory is consumed by, say, art assets, text, and so on, I won't be too worried. As I previously observed in #5945, there are already lots of zombie memory values from the base game that have not been removed. My assumption is that the impact of integer and boolean values is therefore not that high.

2) When a function ends (returns nothing, or an empty value), what happens to the values of the local variables within that function, and their impact on memory?

3) If a function has a large number of local variables (mostly integer/boolean), would this cause a problem? I'm contemplating a reorganization of the GetBestApproachTowardsMajorCiv function that minimizes the number of function calls by caching a lot of values at the start of the function before evaluating conditions.

For e.g. victory competition, this also has benefits in that I can simply set the default value of all the "TheyAreCloseToXVictory" values to false, and only run the checks if victory competition is enabled. Then I don't need to call IsNoVictoryCompetition a bunch of times.

For optional components like C4DF, by caching the value of all relevant values at the start, I only need to check if C4DF is enabled once, allowing me to remove many #if defined and if (MOD_DIPLOMACY_CIV4_FEATURES) checks later in the function.

Draft code attached below: most of these values already exist as variables or function calls within GetBestApproachTowardsMajorCiv. Would this kind of local caching within the function cause problems?

    // Initialize variables and precalculate some expensive checks
    CvDiplomacyAI* pTheirDiploAI = GET_PLAYER(ePlayer).GetDiplomacyAI();
    bool bHuman = GET_PLAYER(ePlayer).isHuman();

    int iEra = GetPlayer()->GetCurrentEra();
    int iTheirEra = GET_PLAYER(ePlayer).GetCurrentEra();
    int iNumCaps = GetPlayer()->GetNumCapitalCities();
    int iNumTheirCaps = GET_PLAYER(ePlayer).GetNumCapitalCities();
    int iNumOurTechs = GET_TEAM(GetPlayer()->getTeam()).GetTeamTechs()->GetNumTechsKnown();
    int iNumTheirTechs = GET_TEAM(GET_PLAYER(ePlayer).getTeam()).GetTeamTechs()->GetNumTechsKnown();
    int iNumTimesCoopWarDenied = GetNumTimesCoopWarDenied(ePlayer);
    int iNumCitiesWeCaptured = pTheirDiploAI->GetNumCitiesCapturedBy(eMyPlayer);
    int iNumCitiesTheyCaptured = GetNumCitiesCapturedBy(ePlayer);
    int iCityDifference = iNumCitiesWeCaptured - iNumCitiesTheyCaptured;
    int iNumCitiesLiberated = GetNumCitiesLiberatedBy(ePlayer);

    bool bBiggestCompetitor = (GetBiggestCompetitor() == ePlayer);
    bool bMajorCompetitor = IsMajorCompetitor(ePlayer);
    bool bCapturedKeyCityFromUs = (IsCapitalCapturedBy(ePlayer) || IsHolyCityCapturedBy(ePlayer));
    bool bCapturedKeyCityFromThem = (pTheirDiploAI->IsCapitalCapturedBy(eMyPlayer) || pTheirDiploAI->IsHolyCityCapturedBy(eMyPlayer));
    bool bDenouncedThem = IsDenouncedPlayer(ePlayer);
    bool bDenouncedUs = pTheirDiploAI->IsDenouncedPlayer(eMyPlayer);
    bool bUntrustworthyFriend = IsUntrustworthyFriend(ePlayer);
    bool bAtWarWithAtLeastOneMajor = MilitaryAIHelpers::IsTestStrategy_AtWar(m_pPlayer, false);
    bool bEasyTarget = IsEasyTarget(ePlayer, false);
    // If at peace with them, they're only an easy target if we're not already at war with somebody else.
    if (!bAtWar)
    {
        bEasyTarget = bEasyTarget && !bAtWarWithAtLeastOneMajor;
    }
    bool bAggressor = false;
    if (bAtWar && GET_TEAM(eTeam).isAggressor(eMyTeam))
    {
        bAggressor = true;
    }

    // Sanity checks
    int iLostGoldPerTurn = CalculateGoldPerTurnLostFromWar(ePlayer, /*bOtherPlayerEstimate*/ false, /*bIgnoreDPs*/ false);
    int iAdjustedGoldPerTurn = GetPlayer()->calculateGoldRate() - iLostGoldPerTurn;

    // Evaluations
    AggressivePostureTypes eExpansionAggressivePosture = GetExpansionAggressivePosture(ePlayer);
    AggressivePostureTypes ePlotBuyingAggressivePosture = GetPlotBuyingAggressivePosture(ePlayer);
    AggressivePostureTypes eMilitaryAggressivePosture = GetMilitaryAggressivePosture(ePlayer);
    BlockLevelTypes eVictoryBlockLevel = GetVictoryBlockLevel(ePlayer);
    DisputeLevelTypes eLandDisputeLevel = GetLandDisputeLevel(ePlayer);
    DisputeLevelTypes eWonderDisputeLevel = GetWonderDisputeLevel(ePlayer);
    DisputeLevelTypes eMinorCivDisputeLevel = GetMinorCivDisputeLevel(ePlayer);
    DisputeLevelTypes eVictoryDisputeLevel = GetVictoryDisputeLevel(ePlayer);
    MajorCivOpinionTypes eOpinion = GetMajorCivOpinion(ePlayer);
    StrengthTypes eMilitaryStrength = GetPlayerMilitaryStrengthComparedToUs(ePlayer);
    StrengthTypes eEconomicStrength = GetPlayerEconomicStrengthComparedToUs(ePlayer);
    TargetValueTypes eTargetValue = GetPlayerTargetValue(ePlayer);
    WarProjectionTypes eWarProjection = GetWarProjection(ePlayer);
    WarStateTypes eWarState = GetWarState(ePlayer);

    // Diplomacy towards us
    MajorCivApproachTypes eVisibleApproach = GetApproachTowardsUsGuess(ePlayer);

    // Victory
    VictoryTypes eSpaceshipVictory = (VictoryTypes) GC.getInfoTypeForString("VICTORY_SPACE_RACE", true);
    VictoryTypes eCulturalVictory = (VictoryTypes) GC.getInfoTypeForString("VICTORY_CULTURAL", true);
    VictoryTypes eDiplomaticVictory = (VictoryTypes) GC.getInfoTypeForString("VICTORY_DIPLOMATIC", true);
    VictoryTypes eDominationVictory = (VictoryTypes) GC.getInfoTypeForString("VICTORY_DOMINATION", true);
    bool bNoVictoryCompetition = IsNoVictoryCompetition();
    bool bWeAreCloseToWorldConquest = IsCloseToDominationVictory();
    bool bWeAreCloseToDiploVictory = IsCloseToDiploVictory();
    bool bWeAreCloseToScienceVictory = IsCloseToSSVictory();
    bool bWeAreCloseToCultureVictory = IsCloseToCultureVictory();
    bool bWeAreCloseToAnyVictoryCondition = IsCloseToAnyVictoryCondition();
    bool bTheyAreCloseToWorldConquest = false;
    bool bTheyAreCloseToDiploVictory = false;
    bool bTheyAreCloseToScienceVictory = false;
    bool bTheyAreCloseToCultureVictory = false;
    bool bTheyAreCloseToAnyVictoryCondition = false;
    if (!bNoVictoryCompetition)
    {
        bTheyAreCloseToWorldConquest = pTheirDiploAI->IsCloseToDominationVictory();
        bTheyAreCloseToDiploVictory = pTheirDiploAI->IsCloseToDiploVictory();
        bTheyAreCloseToScienceVictory = pTheirDiploAI->IsCloseToSSVictory();
        bTheyAreCloseToCultureVictory = pTheirDiploAI->IsCloseToCultureVictory();
        bTheyAreCloseToAnyVictoryCondition = pTheirDiploAI->IsCloseToAnyVictoryCondition();
    }

    // World Congress
    bool bPrimeLeagueCompetitor = false;
    bool bColdWar = false;
    int iBestVotes = 0;
    int iOurLeagueVotes = 0;
    int iTheirLeagueVotes = 0;
    CvLeague* pLeague = GC.getGame().GetGameLeagues()->GetActiveLeague();
    if (pLeague != NULL)
    {
        iOurLeagueVotes = pLeague->CalculateStartingVotesForMember(eMyPlayer, false, true);
        iTheirLeagueVotes = pLeague->CalculateStartingVotesForMember(ePlayer, false, true);

        for (int iPlayerLoop = 0; iPlayerLoop < MAX_MAJOR_CIVS; iPlayerLoop++)
        {
            PlayerTypes eDiploLoopPlayer = (PlayerTypes) iPlayerLoop;

            if (IsPlayerValid(eDiploLoopPlayer))
            {
                if (eDiploLoopPlayer == ePlayer)
                    continue;

                int iVotes = pLeague->CalculateStartingVotesForMember(eDiploLoopPlayer);

                if (iVotes > iBestVotes)
                    iBestVotes = iVotes;
            }
        }
        if (iTheirLeagueVotes > iBestVotes)
        {
            bPrimeLeagueCompetitor = true;
        }

        // City-State Diplomacy
#if defined(MOD_DIPLOMACY_CITYSTATES_RESOLUTIONS)
        if (MOD_DIPLOMACY_CITYSTATES_RESOLUTIONS)
        {
            for (int iPlayerLoop = 0; iPlayerLoop < MAX_MAJOR_CIVS; iPlayerLoop++)
            {
                PlayerTypes eLoopPlayer = (PlayerTypes) iPlayerLoop;
                if (IsPlayerValid(eLoopPlayer))
                {
                    if (GC.getGame().GetGameLeagues()->IsIdeologyEmbargoed(eMyPlayer, eLoopPlayer))
                    {
                        bColdWar = true;
                        break;
                    }
                }
            }
        }
#endif
    }

    // C4DF
    bool bWeAreVassalOfSomeone = false;
    bool bTheyAreVassalOfSomeone = false;
    bool bOurCapitulatedVassal = false;
    bool bOurVoluntaryVassal = false;
    bool bOurCapitulatedMaster = false;
    bool bOurVoluntaryMaster = false;
    bool bAttackedOwnVassal = false;
#if defined(MOD_DIPLOMACY_CIV4_FEATURES)
    if (MOD_DIPLOMACY_CIV4_FEATURES)
    {
        if (GET_TEAM(eMyTeam).IsVassalOfSomeone())
        {
            bWeAreVassalOfSomeone = true;
        }
        if (GET_TEAM(eTeam).IsVassalOfSomeone())
        {
            bTheyAreVassalOfSomeone = true;
        }
        if (GET_TEAM(eMyTeam).GetMaster() == eTeam)
        {
            if (GET_TEAM(eMyTeam).IsVoluntaryVassal(eTeam))
            {
                bOurVoluntaryMaster = true;
            }
            else
            {
                bOurCapitulatedMaster = true;
            }
        }
        else if (GET_TEAM(eTeam).GetMaster() == eMyTeam)
        {
            if (GET_TEAM(eTeam).IsVoluntaryVassal(eMyTeam))
            {
                bOurVoluntaryVassal = true;
            }
            else
            {
                bOurCapitulatedVassal = false;
            }
        }
        for (int iPlayerLoop = 0; iPlayerLoop < MAX_MAJOR_CIVS; iPlayerLoop++)
        {
            PlayerTypes eLoopPlayer = (PlayerTypes) iPlayerLoop;

            if (IsPlayerValid(eLoopPlayer, true) && GET_PLAYER(eLoopPlayer).getTeam() != eTeam)
            {
                if (GET_PLAYER(eLoopPlayer).GetDiplomacyAI()->IsPlayerBrokenVassalAgreement(ePlayer))
                {
                    bAttackedOwnVassal = true;
                    break;
                }
            }
        }
    }
#endif
HungryForFood commented 4 years ago

I only do programming as a hobby, so hopefully my understanding is not too off here. Please correct me if I am wrong.

  1. From what I understand, int takes 4 bytes, while boolean takes 1 byte. Which is not that much individually, and since you've removed a bunch of variables from memory, I don't think it is an issue.

As a side note, I think the impact is higher on certain database tables, where the Firaxis method is to initialise an array to store the values for all possible settings. This is not really relevant to you I think, but might be of interest.

For example, looking at the table which allows traits to give yield per turn to building classes (I think its called Trait_BuildingClassYieldChanges or something similar), if we follow the Firaxis method, for each entry in the Traits table, we will need to define a 3-dimensional array to store all combinations of building class, yield type, and yield. Each of them will be use an int (building class and yield's int will be the id in the database). Let's say you have 40 building classes and 6 yield types defined. This means for each trait, I will have an array which contains 40 × 6 = 240 integers (240 × 4 bytes = 960 bytes, almost a kilobyte) .

This array is initialised, meaning the size will not change. So it will always use the kilobyte, even if in the database, we define nothing. Inside the DLL, it will then be an array full of zeroes.

Usually a trait table might only be used by one trait entry (in our example, only Japan uses it), so this is very wasteful, as out of the 40+ arrays (1 for each trait), only one array might be useful. The other 40 kilobytes are wasted, as they are all zeroes.

What I have been trying with my new tables is to use std::map instead of arrays, and only insert elements when values are non-zero. So, back to our examples, all non-Japan traits will have a std::map with nothing inside. The std::map itself still takes memory though, even if empty, so I still need to check what this actually is, to see if I'm actually doing any saving.

  1. If you only define a variable within the function, and not as a public variable in the header file, the memory will be released once the function ends. The next time the function is called, the game will not know the value from the previous call.

  2. How large is large? Integers and booleans do not use much individually, and the memory is released after the function ends anyway. I think from your example, there are less than 20 integers and booleans each, so 20 × 4 bytes + 20 × 1 bytes = 100 bytes. Doesn't seem much.

ilteroi commented 4 years ago
  1. in general everything we do in the DLL pales in comparison to the memory used for the UI. however you should be careful with large arrays and high multiplicity (eg adding something to each plot which may be instantiated 10000 times vs something added to the diplo AI which exists 64 times). using std::maps in general saves memory compared to arrays which contain mostly zeros, but access is a bit slower (typically not a concern in diplo AI)

  2. local variables live on the stack (a kind of scratch pad). by default max stack size is 1MB iirc. each function has a "stack frame", which is cleared when the function returns. however complex types may internally also do heap allocations (eg a vector<>) so they take only a few bytes on the stack independent of their actual size.

  3. no problem but if the call is cheap and/or the result is only used once, there is little point in having a local cache besides maybe easier debugging. in a release build the compiler will probably optimize it away anyway.

RecursiveStar commented 4 years ago

@HungryForFood @ilteroi Thank you for the reply!