@Askaholic
The thing about performance optimization is it’s extremely weird, counter intuitive, and will almost certainly be a disaster if you don’t collect adequate profiling data at every step of the way.
I've written a profiler that counts how often functions get called. You can find it here:
Counting the time of functions is too unreliably. Even the benchmarks have a 10% - 20% variance even though they perform the exact same code each time. Let alone that the time does not include the garbage collector - and that is exactly the beast you'd want to profile with regard to time.
A other reasons why it is hard to use time instead of just counters:
- Threading
- Waiting
- Crashing
And a screenshot or two:
Which is the reason why there is another topic about ambient sounds.
Which is the reason of this PR: https://github.com/FAForever/fa/pull/3337
If you just start optimizing stuff arbitrarily because a tingling sensation in your tummy tells you it’s a good idea then you’re probably gonna end up wasting a lot of time and see no benefit (but you might end up breaking the game in the process).
I've written dozens of benchmarks to make sure everything I do is actually more performant. You can find them here:
I'm not going in blind here.
IMO what you want to do is gather a bunch of metrics on different possible optimizations and then decide which ones to go with (I think this is what Jip is doing in his main thread).
At this point it is just me performing this type of work. If there would be 3 - 6 people with the right experience working on this then I'd agree with you. But it is just me for now. If I'd take that approach it would take me weeks of analyzing every bit of code and make a proper analysis while meanwhile no work gets done.
So really, we should look at this change in the context of all the other proposed changes, and my guess is we’ll probably find a lot of other stuff that is more beneficial and less dangerous.
I don't agree. I'm trying to find issues that new contributors can pick up easily from various perspectives:
- (1) To have no need to analyze / find something themselves.
- (2) To have to communicate with the other teams, in this case the balance team.
- (3) To have a stepping stone to larger PRs that will benefit way more than this one ever will.
To elaborate:
(1) The FA repository is daunting: there is no test suite of any kind. It is not complete (a lot of the base game files are still in the installation folder). It is hard to navigate at times because of that. And I can not expect people to have the knowledge to understand and find hot spots reliably. Let alone test their code reliably. This issue would be a perfect candidate: just wait for the attack cycle and test again.
(2) I am hoping to attract new contributors at some point that have no history in FAF. At first, it may be daunting to ask the balance team a question because you don't want to sound silly. Or it may be scary to ask for help because you do not want to be perceived as incompetent. This issue would force the contributor to cross that bridge, get to know people a bit better.
(3) I have a personal branch where I did the majority of the projectile files through dangerous regular expressions. See also this branch: https://github.com/Garanas/fa/tree/optimize-effects and see the changes in this PR, or the follow up PRs that again touch dozens of files.
I've described the results of those optimizations (with the units that show the original behavior, correct) in this post where 400 Zthuee all firing continuously were about 30% - 40% cheaper on the sim and I personally experienced less stuttering when comparing it to the standard branch. This would generalize over all projectiles - meaning a large battle would take up a significant smaller portion of the sim and there would be less stutters.
I can't ask a new contributor to start working on this - it is hard to test and hard to get right and the scope is gigantic. But I can ask a new contributor to start on a controlled issue, exactly the one that I am describing in this topic.
Marlo is currently working on a benchmark / test map that should help with testing large changes like these more reliably. Up to that point, performing any major non-isolatable refactoring of the base code is essentially a no-go.
less dangerous
This whole topic is here to discuss the changes and have a careful approach to something that we're all used to see in some way or fashion. That is why there is a topic about ambient sounds (that doesn't receive a lot of attention). And another about the cybran build bots (which was on fire).
I'm not trying to break the game - I'm trying to fix it. And this will require changes that will always show up in some form of another. They'll always be dangerous and potentially game breaking. And I'll always discuss them, exactly as I am doing here.
And as @Azraeel is saying: there is no magic bullet. There are a lot of small 'gotchas' that we can do. And these will just take up hours and hours of careful work over dozens if not hundreds of files. As an example you can view the benchmarks, or read about it here from legit sources:
Especially the latter is quite interesting as it talks about memory usage. One topic that the FA repository has been neglecting from day 1 and have been making quite a bit worse over the years. Understand me right: I'm not blaming anyone. But it is a fact - and that causes stutters that annoy the f out of me when I play the game.
Fixing that will be dangerous too. And I'll do it regardless. And I hope to start a team with people that want to join me on this journey.
With all of that said: please people, I'm not here to break (y)our game. I'm trying to fix it - discuss the changes in topics like these and lets fix it together.