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

XEH - Use compileScript in compileEventHandlers #1467

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

jokoho48
Copy link
Member

@jokoho48 jokoho48 commented Jun 19, 2021

When merged this pull request will:

  • Title
    With this change, mods that still use the old COMPILE_FILE macro still get the loading time speedup from compileScript

@PabstMirror
Copy link
Contributor

IIRC there was almost no difference in speed between the 2 commands for plain old sqf

@jokoho48
Copy link
Member Author

IIRC there was almost no difference in speed between the 2 commands for plain old sqf

from my testsing it is faster to compile a script via compileScript vs what it is when using compile and preprocessfilelinenumbers.

@PabstMirror
Copy link
Contributor

Result: 10.2551 ms
compile preProcessFileLineNumbers "\z\ace\addons\attach\functions\fnc_attach.sqf";  
compile preProcessFileLineNumbers "\z\ace\addons\attach\functions\fnc_canAttach.sqf";  
compile preProcessFileLineNumbers "\z\ace\addons\attach\functions\fnc_canDetach.sqf";  
compile preProcessFileLineNumbers "\z\ace\addons\attach\functions\fnc_detach.sqf";  
compile preProcessFileLineNumbers "\z\ace\addons\attach\functions\fnc_getChildrenActions.sqf"; 


Result:10.3196 ms
compileScript ["\z\ace\addons\attach\functions\fnc_attach.sqf"];  
compileScript ["\z\ace\addons\attach\functions\fnc_canAttach.sqf"];  
compileScript ["\z\ace\addons\attach\functions\fnc_canDetach.sqf"];  
compileScript ["\z\ace\addons\attach\functions\fnc_detach.sqf"];  
compileScript ["\z\ace\addons\attach\functions\fnc_getChildrenActions.sqf"];

one possible benefit would be if someone uses sqfc but doesn't use the new macro, they would get the loading speed bonus of sqfc (but then lose the in-game performance because of stringify). But this just might confuse developers more

@jonpas
Copy link
Member

jonpas commented Jul 23, 2021

Close then as there is no benefit, but rather possible confusion?

@jonpas jonpas changed the title use CompileScript in fnc_compileEventHandlers XEH - Use compileScript in compileEventHandlers Jul 23, 2021
@PabstMirror
Copy link
Contributor

maybe we should wait for 2.06 and test again on that

@jonpas
Copy link
Member

jonpas commented Aug 17, 2023

Did anyone retest this with 2.06? I assume there is no difference, and if so we should close this, runtime performance is more important than load performance.

@PabstMirror
Copy link
Contributor

PabstMirror commented Aug 17, 2023

tested with

a = []; 
b = []; 
  
_baseConfig = configFile;  
private _result = [];  
private _resultNames = [];  
_allowRecompile = true;  
{  
    private _eventName = _x;  
    diag_log text format ["_eventName %1 ", _eventName];  
  
    {  
        private _funcAll = "";  
        private _funcClient = "";  
        private _funcServer = "";  
        private _customName = configName _x;  
  
        if (isClass _x) then {  
              
            private _entry = _x >> "init";  
  
            if (isText _entry) then {  
                _funcAll = getText _entry;  
            };  
  
              
            _entry = _x >> "clientInit";  
            if (isText _entry) then {  
                _funcClient = getText _entry;  
            };  
  
              
            _entry = _x >> "serverInit";  
            if (isText _entry) then {  
                _funcServer = getText _entry;  
            };  
        } else {  
              
            if (isText _x) then {  
                _funcAll = getText _x;  
            };  
        };  
  
        private _eventFuncs = [_funcAll, _funcClient, _funcServer] apply {  
            if (_x == "") then {  
                ;  
                {}   
            } else {  
                ;  
                private _eventFunc = _x;  
  
                // diag_log text format ["_eventFunc %1 ", _eventFunc];  
                  
                if (_allowRecompile) then {  
                    if (toLower (_eventFunc select [0,40]) isEqualTo "call compile preprocessfilelinenumbers '" && {(_eventFunc select [count _eventFunc - 1]) isEqualTo "'"}) then {  
                        private _funcPath = _eventFunc select [40, count _eventFunc - 41];  
  
                          
                          
                        if (_funcPath find "'" == -1) then {  
                            a pushBack compile format ['compile preprocessFileLineNumbers "%1"', _funcPath]; 
                            b pushBack compile format ['compileScript ["%1"]', _funcPath]; 
                            diag_log text format ["%1 is using old", _customName];  
                            _eventFunc = preprocessFileLineNumbers _funcPath;  
                            ;  
                        };  
                    };  
                      
                };  
  
                compile _eventFunc   
            };  
        };  
  
        _result pushBack ["", _eventName, _eventFuncs];  
        _resultNames pushBack _customName;  
    } forEach configProperties [_baseConfig >> format ["Extended_%1_EventHandlers", _eventName]];  
} forEach ["preInit", "postInit"];  

i get almost no difference between { call _x} forEach a and { call _x} forEach b (~200ms on a big modset)

I don't see any harm in merging this though

@jonpas
Copy link
Member

jonpas commented Aug 17, 2023

Is it ready to merge then?

@jonpas jonpas added this to the 3.15.9 milestone Aug 17, 2023
@PabstMirror PabstMirror merged commit 5bdfd2a into master Aug 17, 2023
4 checks passed
@PabstMirror PabstMirror deleted the useCompileScriptInCacheCode branch August 17, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants