1dot13 / source

Source code for the game executable of the Jagged Alliance 2 v1.13 project
101 stars 18 forks source link

Optimise HourlyFactoryUpdate() #12

Open rftrdev opened 2 years ago

rftrdev commented 2 years ago

I've noticed that enabling factories causes a long and very noticeable delay whenever the hour ticks over. After stepping through HandleHourlyUpdate(), the biggest offender by far is this factory function - almost every other hourly update takes 1-2ms, with one or two approaching 10ms, but the HourlyFactoryUpdate() usually takes a whopping 500ms on my machine, even with no items in production.

rftrdev commented 2 years ago

Possible culprit: calling into lua (strategicmap.lua, GetFactoryLeftoverProgress()) way too many times.

Asdow commented 1 year ago

Most of the time seems to be spent on line 13671 SGP_THROW_IFFALSE( _LS.L.EvalFile( filename ), _BS( "Cannot open file: " ) << filename << _BS::cget ); in LuaInitNPCs.cpp

Now, I don't know how lua scripting is supposed to work, but having to read and evaluate scripts/strategicmap.lua from disk on every function call seems silly. Surely there should be a way to cache that, no?

Edit: maybe something like this? I don't know the workings of factories and the lua code enough to say if there are no unintended consequences, but it gets rid of the constant disk access and hickups

INT32 GetFactoryLeftoverProgress( INT16 sSectorX, INT16 sSectorY, INT8 bSectorZ, UINT16 usFacilityType, UINT16 usProductionNumber )
{
    static LuaScopeState _LS( true );
    static bool isInitialized = false;

    // Initialize only once during lifetime of program
    if (!isInitialized)
    {
        isInitialized = true;
        IniFunction( _LS.L(), TRUE );
        IniGlobalGameSetting( _LS.L() );
        const char* filename = "scripts\\strategicmap.lua";
        SGP_THROW_IFFALSE( _LS.L.EvalFile( filename ), _BS( "Cannot open file: " ) << filename << _BS::cget );
    }

    LuaFunction( _LS.L, "GetFactoryLeftoverProgress" ).Param<int>( sSectorX ).Param<int>( sSectorY ).Param<int>( bSectorZ ).Param<int>( usFacilityType ).Param<int>( usProductionNumber ).Call( 5 );

    if ( lua_gettop( _LS.L() ) >= 0 )
    {
        return lua_tointeger( _LS.L(), 1 );
    }

    return 0;
}
Asdow commented 1 year ago

If I understood the code correctly, this should be okay. Moved the IniGlobalGameSetting out of initialization block as it sets global stuff for the LuaScopeState that can change upon starting a new game / loading a save

INT32 GetFactoryLeftoverProgress( INT16 sSectorX, INT16 sSectorY, INT8 bSectorZ, UINT16 usFacilityType, UINT16 usProductionNumber )
{
    static LuaScopeState _LS( true );
    static bool isInitialized = false;

    // Initialize only once during lifetime of program
    if (!isInitialized)
    {
        isInitialized = true;
        IniFunction( _LS.L(), TRUE );
        const char* filename = "scripts\\strategicmap.lua";
        SGP_THROW_IFFALSE( _LS.L.EvalFile( filename ), _BS( "Cannot open file: " ) << filename << _BS::cget );
    }

    IniGlobalGameSetting( _LS.L() );
    LuaFunction( _LS.L, "GetFactoryLeftoverProgress" ).Param<int>( sSectorX ).Param<int>( sSectorY ).Param<int>( bSectorZ ).Param<int>( usFacilityType ).Param<int>( usProductionNumber ).Call( 5 );

    if ( lua_gettop( _LS.L() ) >= 0 )
    {
        return lua_tointeger( _LS.L(), 1 );
    }

    return 0;
}
Asdow commented 7 months ago

Reopening this as my previous solution had to be reverted. See issue #292 The GetFactoryLeftoverProgress kept reading false data from C++ global array gubModderLuaData via lua function call l_GetModderLUAFact, even though debugger showed the actual array contains correct values.

SetFactoryLeftoverProgress was still working correctly, even though it used similar caching of reading and evaluating strategicmap.lua file before the function call..