Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise HourlyFactoryUpdate() #12

Open
rftrdev opened this issue Oct 29, 2022 · 4 comments · Fixed by #32
Open

Optimise HourlyFactoryUpdate() #12

rftrdev opened this issue Oct 29, 2022 · 4 comments · Fixed by #32
Labels
enhancement New feature or request

Comments

@rftrdev
Copy link
Contributor

rftrdev commented Oct 29, 2022

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 rftrdev added the enhancement New feature or request label Oct 29, 2022
@rftrdev
Copy link
Contributor Author

rftrdev commented Oct 29, 2022

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

@Asdow
Copy link
Contributor

Asdow commented Nov 10, 2022

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
Copy link
Contributor

Asdow commented Nov 16, 2022

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
Copy link
Contributor

Asdow commented Mar 16, 2024

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..

@Asdow Asdow reopened this Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants