Improve checking of unit items #6350

mharis001 · 2018-05-17T21:56:14Z

When merged this pull request will:

Add uniqueItems common function - same concept as CBA_fnc_uniqueUnitItems however, caches items if unit is ACE_player (improves performance when checking units with lots of items)
Optimize ace_dogtags_fnc_addDogtagActions by using uniqueItems
Optimize ace_explosives_fnc_getDetonators by using uniqueItems

Performance differences for ace_dogtags_fnc_addDogtagActions:

Items	Old	New
10	0.0683 ms	0.0826 ms
25	0.122249 ms	0.082 ms
50	0.212993 ms	0.0867 ms
75	0.308928 ms	0.0887 ms
100	0.400641 ms	0.0924 ms
300	1.13636 ms	0.0978 ms

Performance differences for ace_explosives_fnc_getDetonators:

Items	Old	New
5	0.0532 ms	0.0945 ms
50	0.189609 ms	0.107308 ms
100	0.335233 ms	0.123047 ms
200	0.626959 ms	0.132293 ms

mharis001 · 2018-05-17T21:59:09Z

Based on slack discussion with @dedmen , unsure whether checking "my_item" in items _unit should be changed as well. Also, probably should use CBA_fnc_uniqueUnitItems rather than _fnc_getItems?

dedmen · 2018-05-18T05:37:51Z

unsure whether checking "my_item" in items _unit should be changed as well.

replacing items has a noticeably positive effect once you go over about 100 item's I'd say. Question is just if we think that the average user carries more than 100 items.
Though that should really be profiled. 100 items is just my gut feeling.

Here are the results to replacing items _player from @mharis001
960 items (from Slack 16th may 11:45AM)

Result: 0.139567 ms Cycles: 7165/10000 Code: "ACE_CableTie" in (items player)
Result: 0.0558 ms Cycles: 10000/10000 Code: "ACE_CableTie" in ([player, false] call CBA_fnc_uniqueUnitItems)

Empty Inventory (from Slack 16th may 11:47AM)

Result: 0.0008 ms Cycles: 10000/10000 Code: "ACE_CableTie" in (items player)
Result: 0.0192 ms Cycles: 10000/10000 Code: "ACE_CableTie" in ([player, false] call CBA_fnc_uniqueUnitItems)

This doesn't include the caching of the new ACE func which probably increases the impact a little more.
And the CBA func in this test still contains assignedItems. So it doesn't return empty array like items does.

Alsoooo... What about https://github.com/acemod/ACE3/blob/master/addons/explosives/functions/fnc_onIncapacitated.sqf#L28 ? :D

dedmen · 2018-05-18T05:40:15Z

addons/common/functions/fnc_uniqueItems.sqf

+params ["_unit"];
+
+private _fnc_getItems = {
+    params ["_unit"];


That is just the CBA thing without assignedItems right? CBATeam/CBA_A3#922 add's a parameter to disable assignedItems. But this specific implementation in here will be faster than the CBA one with it's conditionals.
So duplicate code and faster vs no duplicate code and slower ¯\(ツ)/¯

Btw you don't need params ["_unit"] the local variable implicitly carries over.

Comes down to whether 5 ifs and 1 params are worth duplicating code. 🤔

And you are right about _unit, i guess I added it for cachedCall but it should be available there.

dedmen · 2018-05-18T05:44:25Z

addons/explosives/functions/fnc_getDetonators.sqf

-_result = _result arrayIntersect _result;
-
-_result
+(_unit call EFUNC(common,uniqueItems)) select {getNumber (configFile >> "CfgWeapons" >> _x >> QGVAR(Detonator)) == 1};


here you can again move the configFile >> "CfgWeapons" outside the loop.
How often does this function run? That's a interaction menu condition right? so probably worth it.

Keep forgetting to do that.. It's called for interaction menu actions, inventory EHs, and canDetonate (so lots).

dedmen

https://github.com/acemod/ACE3/pull/6350/files#r189167878 (my last comment) I want that :u

mharis001 · 2018-05-18T06:23:01Z

onIncapacitated definitely has to get changed, just need to decide between using items command or function for in.

mharis001 · 2018-08-30T01:19:01Z

Replacing the cachedCall part (0.0118 ms):

[_unit, _fnc_getItems, _unit, QGVAR(uniqueItemsCache), ITEMS_CACHE_TIME, "cba_events_loadoutEvent"] call FUNC(cachedCall);

with something like this (0.0065 ms):

private _items = _unit getVariable QGVAR(uniqueItemsCache);
if (isNil "_items") then {
    _items = call _fnc_getItems;
    _unit setVariable [QGVAR(uniqueItemsCache), _items];
};
+_items

and a loadout event added in postInit to clear the cache seems worth it, considering we are going for overkill.

This was tested with a low item count (4 unique items), the situation in which the items (~0.0013 ms) command is faster than this solution. This change would help decrease the difference.

And for similar reasons, I think _fnc_getItems should be used rather than CBA_fnc_uniqueUnitItems.

dedmen · 2018-08-30T11:06:38Z

Keep in mind that loadout event only works for local player. the _unit there makes it look like it works everywhere. Might just use ACE_Player there for readability sake.

how much faster is _fnc_getItems..
But I'd say every microsecond here is worth it. Considering how often it is being called in interaction menu. So _fnc_getItems is probably more than worth it :D

mharis001 · 2018-09-16T00:11:15Z

With no caching at all (4 items): _fnc_getItems (0.0085 ms) vs [_unit, false, true, true, true, false] call CBA_fnc_uniqueUnitItems (0.0241 ms), about 3x faster.

I think the uniqueItems function is as optimized as it will get now, just have to go through and replace any items _player that are left.

mharis001 · 2018-09-16T03:37:15Z

I think this PR is complete (replaced most/all items command uses), ready for review.

dedmen

no further comment.

dedmen

lol I can approve multiple times?

* Add uniqueItems function * Optimize dogtag children actions function * Optimize getDetonators item search * Store CfgWeapons lookup in getDetonators * Update items to use new function * Update items to use new function 2 * More optimization of uniqueItems function * Update items to use new function 3

mharis001 added 3 commits May 17, 2018 14:37

Add uniqueItems function

319e7f5

Optimize dogtag children actions function

35fe64b

Optimize getDetonators item search

19c8880

dedmen reviewed May 18, 2018

View reviewed changes

dedmen suggested changes May 18, 2018

View reviewed changes

Store CfgWeapons lookup in getDetonators

5443314

PabstMirror added the kind/enhancement Release Notes: **IMPROVED:** label Jun 1, 2018

PabstMirror added this to the Ongoing milestone Jun 1, 2018

PabstMirror modified the milestones: Ongoing, 3.13.0 Jun 25, 2018

mharis001 added 3 commits July 1, 2018 00:16

Merge branch 'master' into optimize-unit-items

084bdaa

Update items to use new function

882f8de

Update items to use new function 2

bc5a2ef

More optimization of uniqueItems function

12046bc

mharis001 added 2 commits September 15, 2018 23:19

Merge branch 'master' into optimize-unit-items

79b556f

Update items to use new function 3

a0cab46

mharis001 force-pushed the optimize-unit-items branch from 7de3c20 to a0cab46 Compare September 16, 2018 03:31

dedmen approved these changes Sep 16, 2018

View reviewed changes

PabstMirror approved these changes Sep 17, 2018

View reviewed changes

PabstMirror merged commit 803d497 into acemod:master Sep 17, 2018

mharis001 deleted the optimize-unit-items branch September 17, 2018 22:46

PabstMirror modified the milestones: 3.13.0, 3.12.4 Nov 9, 2018

jonpas mentioned this pull request Mar 4, 2019

Add Ground Spike Antenna IDI-Systems/acre2#639

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve checking of unit items #6350

Improve checking of unit items #6350

mharis001 commented May 17, 2018

mharis001 commented May 17, 2018 •

edited

Loading

dedmen commented May 18, 2018 •

edited

Loading

dedmen May 18, 2018 •

edited

Loading

mharis001 May 18, 2018 •

edited

Loading

mharis001 May 18, 2018 •

edited

Loading

dedmen May 18, 2018

mharis001 May 18, 2018 •

edited

Loading

dedmen left a comment •

edited

Loading

mharis001 commented May 18, 2018

mharis001 commented Aug 30, 2018

dedmen commented Aug 30, 2018 •

edited

Loading

mharis001 commented Sep 16, 2018

mharis001 commented Sep 16, 2018

dedmen left a comment

dedmen left a comment

Improve checking of unit items #6350

Improve checking of unit items #6350

Conversation

mharis001 commented May 17, 2018

mharis001 commented May 17, 2018 • edited Loading

dedmen commented May 18, 2018 • edited Loading

dedmen May 18, 2018 • edited Loading

Choose a reason for hiding this comment

mharis001 May 18, 2018 • edited Loading

Choose a reason for hiding this comment

mharis001 May 18, 2018 • edited Loading

Choose a reason for hiding this comment

dedmen May 18, 2018

Choose a reason for hiding this comment

mharis001 May 18, 2018 • edited Loading

Choose a reason for hiding this comment

dedmen left a comment • edited Loading

Choose a reason for hiding this comment

mharis001 commented May 18, 2018

mharis001 commented Aug 30, 2018

dedmen commented Aug 30, 2018 • edited Loading

mharis001 commented Sep 16, 2018

mharis001 commented Sep 16, 2018

dedmen left a comment

Choose a reason for hiding this comment

dedmen left a comment

Choose a reason for hiding this comment

mharis001 commented May 17, 2018 •

edited

Loading

dedmen commented May 18, 2018 •

edited

Loading

dedmen May 18, 2018 •

edited

Loading

mharis001 May 18, 2018 •

edited

Loading

mharis001 May 18, 2018 •

edited

Loading

mharis001 May 18, 2018 •

edited

Loading

dedmen left a comment •

edited

Loading

dedmen commented Aug 30, 2018 •

edited

Loading