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

Cache config UI #4234

Merged
merged 6 commits into from
Aug 18, 2016
Merged

Cache config UI #4234

merged 6 commits into from
Aug 18, 2016

Conversation

jonpas
Copy link
Member

@jonpas jonpas commented Aug 11, 2016

When merged this pull request will:

  • Cache all selective UI config at mission start using CBA namespace
  • Make compile only be called once per config on mission start instead of on every infoDisplayChanged and settingChanged event
  • ~33% improvement in fnc_setAdvancedElement (~0.09 ms -> ~0.062 ms)

@jonpas jonpas added the kind/optimization Release Notes: **IMPROVED:** label Aug 11, 2016
@jonpas jonpas added this to the 3.7.0 milestone Aug 11, 2016
@jonpas
Copy link
Member Author

jonpas commented Aug 11, 2016

Just did a test and for some reason this is actually a bit slower. Calling ace_ui_fnc_setAdvancedElement results in the following, even though by common sense it should be the function to get main improvement from:
Old: ~0.052 ms
New: ~0.072 ms

I am a tad bit confused honestly. Main goal was to prevent compile taking up precious execution time, and it's not select being slow either.

EDIT: And the element I was testing this with has no conditions specified (empty class), so compile can't be the case.

@jonpas
Copy link
Member Author

jonpas commented Aug 11, 2016

Was using wrong argument in the test case, proper test case shows (ammoCount which also has one condition "(false)"):
Old: ~0.09 ms
New: ~0.115 ms
New without select: ~0.07 ms

So select does overall make it slower, which kinda defeats the purpose of this PR. I imagine with many conditions this way would eventually pay off, but not with the 2 we have now.

@nicolasbadano
Copy link
Contributor

I'm under the impression that BIS sped up reading from config. While fiddling the other day it seemed like caching a config entry on a namespace vs reading it from config wasn't much different anymore.

It should be properly confirmed though. I bet there're perf numbers for config reads buried somewhere among old PRs.

@jonpas
Copy link
Member Author

jonpas commented Aug 12, 2016

If that's true then the only reason to do any caching is due to compile, which in this case is minimal as we don't have many conditions that have to be evaluated all the time. I am going to try another version with just compile cache and add some more conditions to get somewhat realistic numbers.

@jonpas
Copy link
Member Author

jonpas commented Aug 16, 2016

  • Changed to use CBA Namespaces
  • Improved setAdvancedElement function speed
  • Simplified scripted API function (namespaces mostly)

Over-all, I got the setAdvancedElement function to run in ~0.062 ms with current set of conditions (same test as above). May not be much of a difference now, but keep in mind this function can run up to 3 times (in one frame) when entering/exiting a vehicle or changing seat, and with more conditions this should help quite a lot.

This is good to go! 👍

@commy2
Copy link
Contributor

commy2 commented Aug 16, 2016

I'm under the impression that BIS sped up reading from config. While fiddling the other day it seemed like caching a config entry on a namespace vs reading it from config wasn't much different anymore.

That's a tough one. Configs are read from disk, so it really depends on your setup.

0.09 -> 0.062 is a ~33% improvement. Good job 👍

@jonpas
Copy link
Member Author

jonpas commented Aug 17, 2016

Configs are read from disk, so it really depends on your setup.

Are you sure? Game compiles them on start, I would hope they are in memory.

@nicolasbadano
Copy link
Contributor

I would hope they are in memory.

Yeah, I think so too. My guess was that they were in memory, but structured in such a way that accessing them was not performant. If that's the case they might have changed the implementation.

@nicolasbadano
Copy link
Contributor

As far as I can tell, Arma used to read all config entries and store them as a tree. Each node of the tree is a config class, that contains all their children. Children are accessed by name using a hashtable. That implementation seems to make sense, and shouldn't be too slow.

IDK why we used to measure such poor performance at some point, but I'm quite sure that was the case.

@jonpas
Copy link
Member Author

jonpas commented Aug 17, 2016

That's good to know, would be nice if we could get some definite answer on it, but I am not sure that's likely.

@jonpas jonpas merged commit 986ac43 into master Aug 18, 2016
@jonpas jonpas deleted the cacheUI branch August 18, 2016 17:07
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.

3 participants