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

Arsenal - Improve performance of loadout verification #9316

Merged

Conversation

LinkIsGrim
Copy link
Contributor

@LinkIsGrim LinkIsGrim commented Aug 10, 2023

When merged this pull request will:

  • Title.
  • Remove amount of unavailable/nullItems, list is checked directly instead.

Switched to single iteration through the loadout array. See previous optimizations below.

Old

Optimization 1: Add items to nullItemsList along with configCase changes, since we know that `configName _x == ""` means the class doesn't exist anywhere.

Optimization 2: Replace null items with empty string directly when changing loadout contents to configCase. This removes the need to check for isClass _x on the second iteration through the loadout.

Optimization 3: Check all classnames against GVAR(virtualItemsFlatAll) instead of categories, since hashmap lookup time is constant. This allowed for removing the index checking in _fnc_weaponCheck and others.

Optimization 4: Change use of pushBackUnique to combo pushBack and arrayIntersect as that is faster in most cases now, particularly with large arrays.

Optimization 5: Switch use of count of unavailable/null items for checking the lists against a default value in relevant places.

Optimization 6: Replaced some loop logic (assignedItems now uses forEach, and container item checking was changed to use the _containerItems array already defined in _x params).

Optimization 7: Due to all of the above, config lookups and some variable assignments could be removed.

To make this work, the unavailable items list needs to be checked against [""]. Otherwise array modification or explicit checking for _x != "" would be needed for everything, which would reduce performance gain.

End result, loading just CBA and ACE and checking against a full arsenal, is a 13% improvement:


testLoadout = [[["rhs_weap_m4a1_blockII_KAC_bk","rhsusf_acc_nt4_black","","rhsusf_acc_su230",["rhs_mag_30Rnd_556x45_Mk262_Stanag",30],[],"rhsusf_acc_grip2"],["rhs_weap_M136_hp","","","",[],[],""],["rhsusf_weap_glock17g4","","","",["rhsusf_mag_17Rnd_9x19_JHP",17],[],""],["VSM_Multicam_Crye_SS_od_shirt_Camo",[["ACE_CableTie",2],["ACE_EarPlugs",2],["ACE_EntrenchingTool",1],["ACE_epinephrine",3],["ACE_RangeCard",1],["ACE_splint",4],["ACE_tourniquet",4],["ACE_M26_Clacker",1],["ACE_MapTools",1],["ACE_Flashlight_XL50",1],["ACE_packingBandage",15],["ACE_IR_Strobe_Item",1],["ACE_morphine",3]]],["VSM_FAPC_Breacher_OGA_OD",[["rhs_mag_30Rnd_556x45_Mk262_Stanag",8,30],["HandGrenade",2,1],["ACE_M84",4,1],["rhs_mag_an_m14_th3",2,1],["SmokeShell",4,1],["SmokeShellBlue",1,1],["SmokeShellGreen",1,1],["SmokeShellRed",1,1],["rhsusf_mag_17Rnd_9x19_JHP",4,17]]],["VSM_OGA_OD_Backpack_Compact",[["DemoCharge_Remote_Mag",2,1]]],"VSM_OD_spray_OPS","VSM_Shemagh_Balaclava2_OD_Peltor_Goggles",["Laserdesignator_01_khk_F","","","",["Laserbatteries",1],[],""],["ItemMap","B_UavTerminal","ItemRadio","ItemCompass","ACE_Altimeter","ACE_NVG_Wide"]],[]]

Result:
0.768012 ms

Cycles:
1303/10000

Code:
[testLoadout] call ace_arsenal_fnc_verifyLoadoutOld

Result:
0.676349 ms

Cycles:
1479/10000

Code:
[testLoadout] call ace_arsenal_fnc_verifyLoadout

Loadout verification is already pretty fast now thanks to hashmaps but it's still the part of the arsenal where a user is most likely to notice stuttering or hang ups, particularly with large loadout lists and loadouts with many different item types.

Possible issues:

  • Items with the same classname in different config types (CfgWeapons and CfgVehicles, for example) could end up not getting registered as null. This is going to be extremely rare, though.
  • Third parties might rely on the output of FUNC(verifyLoadout)? It's not public, though.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@LinkIsGrim LinkIsGrim added the kind/optimization Release Notes: **IMPROVED:** label Aug 10, 2023
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Mostly formatting changes in my review, but also some a very minor performance improvement suggestion.

addons/arsenal/functions/fnc_verifyLoadout.sqf Outdated Show resolved Hide resolved
@johnb432
Copy link
Contributor

Optimization 4: Change use of pushBackUnique to combo pushBack and arrayIntersect as that is faster in most cases now, particularly with large arrays.

Are you sure about this? I had the impression the opposite was true.
That's neat information for the future if true.

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

Yeah, that is true IIRC. With pushBackUnique you check if the value already exists (so you iterate through the whole thing) each time you call it. With pushBack you don't do that, and you only do it once with arrayIntersect at the end.

@LinkIsGrim
Copy link
Contributor Author

Mostly formatting changes in my review, but also some a very minor performance improvement suggestion.

I was under the assumption that creating a hashmap/setting a key is slower than using arrays, and I was specifically avoiding using - [""]. Have you profiled that to see if it's actually faster?

@johnb432
Copy link
Contributor

johnb432 commented Aug 12, 2023

I was under the assumption that creating a hashmap/setting a key is slower than using arrays, and I was specifically avoiding using - [""]. Have you profiled that to see if it's actually faster?

It really depends on what is being done:

Hashmaps will be roughly 80-90x faster if there are loads of different entries, like here:

private _nullItemsList = createHashMap;

for "_i" from 1 to 10000 do {
	_nullItemsList set [_i, nil];
};

keys _nullItemsList

Arrays will be 2x faster if there are loads of the same entry:

private _nullItemsList = [];

for "_i" from 1 to 10000 do {
	_nullItemsList pushback 1; // Not _i like in example above!
};

_nullItemsList arrayIntersect _nullItemsList

I'm guessing in this application it's more of the first case, however in game it doesn't make a measurable difference.
I'll edit/remove my suggestions if you want to stick with an array.


If a loadout were to have all items available _unavailableItemsList wouldn't contain "", so it has to be removed. Also feels like whack to have to check for [""] specifically instead of [] imo.

@johnb432
Copy link
Contributor

If a loadout were to have all items available _unavailableItemsList wouldn't contain "", so it has to be removed. Also feels like whack to have to check for [""] specifically instead of [] imo.

Updated my suggestions @LinkIsGrim

@LinkIsGrim
Copy link
Contributor Author

LinkIsGrim commented Aug 13, 2023

LinkIsGrim@e5950f2 switches to a single iteration through the loadout array (turning everything into configCase, checking for null and availability all at once).

Output for the same test loadout:

Result:
0.676349 ms

Cycles:
1479/10000

Code:
[testLoadout] call ace_arsenal_fnc_verifyLoadout

Readability isn't the best, though.

LinkIsGrim and others added 2 commits September 2, 2023 00:54
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
Co-authored-by: johnb432 <58661205+johnb432@users.noreply.github.com>
Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

I've tested it, but I feel it could do with some more by others.

@LinkIsGrim LinkIsGrim merged commit 6928adf into acemod:master Feb 10, 2024
5 checks passed
@LinkIsGrim LinkIsGrim deleted the arsenal-verifyLoadouts-performance1 branch February 10, 2024 15:58
@LinkIsGrim LinkIsGrim added this to the 3.16.4 milestone Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/optimization Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants