Lots of time spent in GetAnyBodyProgress due to Enum.GetValues

[abrenneke]

I'm new to KSP modding so apologies if this is just my system, or because of a debug build, or already known, etc!

I ran a profiler on my (heavily modded) KSP installation, and one vanilla function stood out to me, GetAnyBodyProgress. The function seems simple, but a lot of time is just spent calling Enum.GetValues(typeof(ProgressType)) every time. If this was simply cached, I'd imagine the function would speed up a bunch.

Rough decompiled vanilla code:

ProgressType[] values = (ProgressType[]) Enum.GetValues(typeof (ProgressType));
ProgressMilestone progressMilestone = new ProgressMilestone(body, ProgressType.ORBIT, manned);
int length = values.Length;
while (length-- > 0)
{
  progressMilestone.type = values[length];

This is when I was in the tracking station (was trying to see why that was particularly slow for me).

Seems like a potentially simple fix - I've done Harmony patching for RimWorld before.

[NathanKell]

That's a great find! Are you up to making a PR? It looks like it'd be a fairly simple transpiler patch, just replace the Enum.GetValues() call with a ldfld and a bunch of noops.

[NathanKell]

EDIT: And store the result in a static readonly field in the patch class to ldfld from, obvs.

[abrenneke]

Yeah, I can give it a shot

[NathanKell]

Awesome! Thank you! Should be quite easy, you can probably just duplicate the Strategy Duration patch, move the cs to Performance and rename the filename/class, add a new entry to settings.cfg in performance and set it true, and then change the code to have just one transpiler that hits the method in question. Holler if I can help further! :)

[gotmachine]

Unrelated, but what toolset are you using for profiling ? Those screenshots don't look like the Unity profiler...

[abrenneke]

Haven't had any time to work on this yet so if you have free time go ahead and implement!

I use DotTrace for profiling, more habit than anything else probably, it has built in Unity support.

[gotmachine]

Took me a bit of time to actually find a contract triggering calls to GetAnyBodyProgress(). The method is used in the update loop only by a few stock contracts added in the latest KSP updates : rover construction and comet sample. In all other other contracts it is only called once at contract generation.

At least in the rover construction contract (didn't check the comet sample) it is called once per body per update, so the overhead is indeed significant (especially if you have a larger than stock body count).

Turning the Enum.GetValues() call into an access to a static field improve the time per call to GetAnyBodyProgress() from 0.036ms to 0.001ms.

For the rover construction contract in a non-debug stock install, per frame overhead of the method went from 0.497 ms to 0.016 ms, so definitely a worthy fix.

Beside the Enum.GetValue() overhead, the method is uselessly throwing away a new instance of the ProgressMilestone class on every call, generating 0.6KB of garbage per frame for the rover construction contract. Not very significant, so not really worth a fix.

[abrenneke]

Oh only the two contracts, well still that's a big gain if someone has one being offered! Thank you a bunch for fixing, sorry I never got to it myself!