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

Compile and cache configFile eventhandlers at preStart #1052

Merged
merged 12 commits into from
Feb 17, 2019

Conversation

dedmen
Copy link
Contributor

@dedmen dedmen commented Feb 8, 2019

They won't change after preStart as the config cannot be edited. So no reason to recompile them each preInit.

This is a mission with no eventhandlers in description or campaign config. Meaning 100% of that compileEventHandlers is just configFile. 670ms wasted each preInit for something that can be done once.

@dedmen
Copy link
Contributor Author

dedmen commented Feb 8, 2019

Before:
Sorry for crude, profiler is bugging around
tracy_2019-02-08_12-17-49

INCON is
https://github.com/CBATeam/CBA_A3/blob/master/addons/xeh/fnc_preInit.sqf#L75-L85
XEH is that https://github.com/CBATeam/CBA_A3/blob/master/addons/xeh/fnc_preInit.sqf#L89-L99
INCON about 700ms
XEH about 1300ms
both at preInit.

After:

tracy_2019-02-08_11-42-14

620ms for incon.
1200ms for compileEH. Both at preStart.
And preInit:
tracy_2019-02-08_12-22-34

Whole XEH_preInit only takes 7.5+2.6ms (profiler messed up) now instead of 2000ms.
incon is at 1.7ms (the profiler messed up the measurement)
And compileEH is 2.6ms

@commy2 commy2 added this to the 3.10 milestone Feb 8, 2019
@dedmen dedmen changed the title WIP: Compile and cache configFile eventhandlers at preStart Compile and cache configFile eventhandlers at preStart Feb 8, 2019
Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Pabst wrote, this needs a rewrite of CBA_fnc_compileEventHandlers to compile the sum of all preInit and postInit events in such a way, that they include the isServer etc. checks in their code.
Otherwise serverInit events would run on a dedicated client, and clientInit events would run on a (dedicated?) server.
Otherwise this PR is fine.

@commy2 commy2 added WIP and removed Needs Testing labels Feb 9, 2019
Copy link
Contributor

@commy2 commy2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

@PabstMirror
Copy link
Contributor

rough idea: 06ad425
compile server/client xeh seperately and selectivly add at mission time
I skipped pre/postInit events for now because that won't really effect performance

@commy2
Copy link
Contributor

commy2 commented Feb 15, 2019

Anyone update this PR, or should I make my own with both ideas?
This is the last one we need for 3.10 I'd say.

@dedmen
Copy link
Contributor Author

dedmen commented Feb 15, 2019

Can't you just push into this one? feel free to. I won't have time till monday

@commy2
Copy link
Contributor

commy2 commented Feb 15, 2019

How?

@PabstMirror
Copy link
Contributor

git push https://github.com/dedmen/CBA_A3.git YourLocalBranchName:cacheConfigCompileEH

@PabstMirror
Copy link
Contributor

pushed changes for the init events as well

if (!(_funcAll isEqualTo {})) then { is pretty much the same as [] call {},
so we could remove those checks (for the init, not classEvents) if that looks better

preProcessing is disabled with the recompile bit or filePatching to prevent problems with developers
also added some debugging for funcs that can be precompiled

XEH: Could not recompile [postInit-cba_versioning]: if !(isServer) then {call compile preprocessFileLineNumbers '\x\cba\addons\versioning\XEH_postInitClient.sqf'}

@commy2
Copy link
Contributor

commy2 commented Feb 16, 2019

That didn't work well. Now the diff shows commits between when this branch was split from master and not the current master.

@@ -15,5 +15,5 @@ if !(_event isEqualTo "") then {
private ["_event", "_className"]; // prevent these variables from being overwritten
_args call compile getText _x;
} forEach configProperties [_x >> XEH_FORMAT_CONFIG_NAME(_event) >> _className, "isText _x"];
} forEach XEH_MAIN_CONFIGS;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Macro seemed pointless. Bugged me for a while and now is an opportunity to muck it out.

@commy2
Copy link
Contributor

commy2 commented Feb 17, 2019

By sorting the events in global/client/server on preStart in 3 different functions, we change the order in which these functions are executed. It used to be in order they appear in config regardless whether it was a global or server event. The only thing that mattered was load order.
Now all global ones are executed first, then all client ones, then all server ones.

@PabstMirror
Copy link
Contributor

class Extended_PostInit_EventHandlers {
    class test1 {
        init = "diag_log 'test1 - init'";
        serverInit = "diag_log 'test1 - serverInit'";
    };    
    class test2 {
        init = "diag_log 'test2 - init'";
        serverInit = "diag_log 'test2 - serverInit'";
    };
    class test3 {
        init = "diag_log 'test3 - init'";
        serverInit = "diag_log 'test3 - serverInit'";
    };
    test4 = "diag_log 'test4 - init'";
};

"test1 - init"
"test1 - serverInit"
"test2 - init"
"test2 - serverInit"
"test3 - init"
"test3 - serverInit"
"test4 - init"

@commy2 commy2 merged commit 76a09cb into CBATeam:master Feb 17, 2019
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