Questions about performance: tactical missiles

Azraaa

@askaholic said in Questions about performance: tactical missiles:

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. One optimization can easily gain you more performance than hundreds of other ones combined if you prioritize correctly. 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). 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). 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 feel like you are looking for the miracle change in the core which does not exist, it is a hundred changes that will improve the performance my guy, its not a magic bullet. You'll have to optimize units anyhow you cant dodge that, and THIS is a main issue. Yes there are obvious optimizations that make a difference but nothing is a huge magic bullet to performance.
I stress that with importance that there is in-fact no magic bullet to solving performance, and combat & balance related things will have to undergo the telescope just like everything else.

Jip

@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:

https://gitlab.com/supreme-commander-forged-alliance/other/profiler

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:

In repository: https://github.com/FAForever/fa/tree/deploy/fafdevelop/lua/benchmarking/benchmarks
In profiler (that can run them): https://gitlab.com/supreme-commander-forged-alliance/other/profiler/-/tree/main/mods/profiler/modules/benchmarking/benchmarks

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.

snoog

You're doing awesome work Jip, thank you for it. Unfortunately it is the internet and in particular, gaming. There's always gonna be people who get worked up over any kind of change to "their" game whether it's for the best or not.

I'd love to start contributing, but I have too much on my plate right now as it is and I don't even have experience in the languages FAF uses, like LUA. But it'll definitely be on my mind to try at some point.

Jip

@snagglefox

I think you're drawing a conclusion too fast. The points Askaholic makes are very accurate. My post only describes the vision I have for an performance issue like this one.

I'd love to start contributing, but I have too much on my plate right now as it is and I don't even have experience in the languages FAF uses, like LUA. But it'll definitely be on my mind to try at some point.

I understand - luckily there are more ways to contribute. Just playing the develop branch and reporting back is part of that and highly appreciated.

Askaholic

I’m not sure why you all are putting so many words in my mouth. I’m not talking about magic bullets or people trying to break the game. I’m talking about making intelligent data driven decisions with a consideration of the system as a whole and not just one piece at a time.

What I think you should do is make a big table with all of the proposed changes, their possible gains and the drawbacks of doing them. Then they can be ranked and prioritized and you can knock out the low hanging fruit before even worrying about stuff that could potentially break the balance.

snoog

@askaholic said in Questions about performance: tactical missiles:

I’m not sure why you all are putting so many words in my mouth. I’m not talking about magic bullets or people trying to break the game. I’m talking about making intelligent data driven decisions with a consideration of the system as a whole and not just one piece at a time.

What I think you should do is make a big table with all of the proposed changes, their possible gains and the drawbacks of doing them. Then they can be ranked and prioritized and you can knock out the low hanging fruit before even worrying about stuff that could potentially break the balance.

That kind of stuff requires a team really. As it is, Jip made some tools to help him find some performance issues and brings them to our attention to discuss and for him or others to fix. To do it your way with just him or even a couple helpers would take forever to discover most or all things fixable and study their performance drawbacks and possible fixes.

Askaholic

No it doesn’t. Jip already has written a bunch of benchmarks (which he linked above) and in each of these threads he’s posted some timing information. Just compile this data into a single table and you will already be eliminating so much guesswork. It’s a simple organizational thing that will help you gain a lot more insight out of the data that you’ve already collected by looking at the system as a whole rather that just the individual pieces and then forgetting about them.

Jip

I’m not sure why you all are putting so many words in my mouth. I’m not talking about magic bullets or people trying to break the game.

My apologies - that was not my intent.

Jip already has written a bunch of benchmarks (which he linked above) and in each of these threads he’s posted some timing information.

Knowing about them didn't require a team. Applying them certainly does - as it involves changes in hundreds of files and many of those are hard to test if it is your first time helping. To give an idea:

Each projectile can have its own logic.
Each unit can have its own logic.
Each weapon can have its own logic.

Take for example the applying of this benchmark: https://gitlab.com/supreme-commander-forged-alliance/other/profiler/-/blob/main/mods/profiler/modules/benchmarking/benchmarks/function-scope.lua

That is applicable to almost all files in the repository. It isn't difficult to do. It is a lot of information to take in however. And it is just hard to test at times making it tricky for a novel contributor as he'll likely just make typo's.

I know that last bit because I've done programming sessions with two friends who did not have experience with Lua at the time, but they did have some experience in general. The results are Genesis of the Order Survival and Survival Stranded. I even made snippets for them and in specific of the syntax of Lua because they kept writing it wrong, taking up tons of time because some bits of their code was hard to run.

I don't know what to add to defend this change. If people seriously disagree then I'll likely just ask a moderator to close this topic in the future.

Valki

Can't you put this, despite minor balance disruptions, in a ranked mod?

Then you can really start collecting data.

maudlin27

If it can be implemented without changing how high the missiles go then I’d see it as a good change. My main concern (already mentioned above by ofhers) is maps like astro craters where even a slight change in the height of a missile from a cruiser could have a dramatic balance impact (at the moment some but not all missiles can clear the astro cliffs so i expect even a small change could have a large impact in the damage cruisers could do)

Jip

@valki said in Questions about performance: tactical missiles:

Can't you put this, despite minor balance disruptions, in a ranked mod?

Then you can really start collecting data.

I will not do this - for the same reasons I did not do it for the Cybran drones. This is supposed to be a small issue on Github that new contributors can use as a stepping stone. Adding the creation of a mod that then gets played two times isn't worth the overhead it causes.

kdrafa91

@jip I wish i understood computers to help you. If you fix pathfinding you will be my hero

veteranashe

Do the t3 sub Cruse missiles shoot different as well?

Jip

@veteranashe said in Questions about performance: tactical missiles:

Do the t3 sub Cruse missiles shoot different as well?

They would likely be affected by this change too - yes. But, for now this change is far away from being part of the game.

@Askaholic As an example of a significant performance improvement: https://github.com/FAForever/fa/pull/3392 . This branch includes the tactical missile changes for the cruiser (but any tactical missile launcher will have to deal with the changes in the end) and the changes applied to the Zthuee as described in the original post about performance.

I've described the changes and why they were made in the PR. The behavior of the Zthuee remain untouched - they work exactly the same. A small copy from the PR:

When having 400 Zthuee idling we spent 3 ms / tick on both branches. When we let them ground fire, the results are:

Current default FAF branch: 11 / 13 ms -> 8 / 10 ms for firing
This branch: 8 / 9 ms -> 5 / 6 ms for firing

Disclaimer: make sure you are completely zoomed out when checking the performance of the sim. The amount of effects Zthuee spawn do effect the sim and the results if they get rendered.

This shows that there is a significant gain to be had for applying the benchmark optimizations to projectiles in general. This includes:

No empty and / or unnecessary table allocations
Upvalue moho functions
Pre-allocating table elements
Inlining of functions

Using the benchmarks as a guideline in combination with the profiler as to how often things are called. These changes are a lot less straightforward.