stock VesselDeltaV.FixedUpdate is very expensive

[JonnyOThan]

In debug mode, I'm frequently seeing this spike to 140ms+ when it runs. It seems like it's timesliced to run every 5 frames.

Oddly, this also seems to run while in timewarp and resources are not changing.

[gotmachine]

Oddly, this also seems to run while in timewarp and resources are not changing.

Stock resource generators and converters are running as usual during timewarp, and many plugins are doing it as well, so resources can and will change. I agree that updating dv stats while engines cannot in theory be used isn't very useful, but there are a handful of plugins attempting to do such a thing, so even trough they are unlikely to be relying on the dv stats to function, the user still might.

From memory, I was under the impression that the not so great perf characteristics of the stock dv calcs was mainly due to the constant reallocation of a large amount of big temporary objects, which could be alleviated by using object pools, but I just did a quick check and allocations seem to actually account for the less than 7-8 % of the overall call time. Profiling also show that the call time only seem to really spike when engines are actually burning, but yeah, they did a good job at hiding the overhead but running it only every 5 frames, in my quick test it is costing a good 20% of the frame time when it runs.

Skimming through the code, there are definitely a lot of tiny or not so tiny things that could optimized in a way or another, but this is a huge implementation with a lot of intermediate objects involved and its hard to judge if doing a micro-optimization pass on the methods body would have a significant effect or if the main bottlenecks are more structural. A first step would be to insert profiler markers or stopwatches everywhere to get a more accurate picture of where the time is actually spent.

But in the end, I fear any significant optimization would require basically rewriting the whole thing, with the constraint that we might not want to deviate too much from the how the thing is structured, as the intermediate objects are publicly exposed and used in various ways.

[JonnyOThan]

This makes sense, and this method in particular seems much worse in the debug build than release. It may also be very craft-specific (while many large craft are flown on my stream, they might have different makeup of modules or connectivity between parts).

[siimav]

Maybe the better idea here is to replace it with stats from MJ or KER. So when either of those mods is installed then do not run stock calcs at all and fetch the stats from those mods instead. I imagine using reflection here would be a real pain though so would need to actually be implemented in the corresponding mods themselves. Or at least have some support code to make KSPCF integration easier.

[gotmachine]

Having MJ and KER override the stock backend would indeed be ideal, but I don't see the value of having KSPCF as a middleman to achieve that...

[Poodmund]

This kind of(?) already exists with Basic DeltaV: https://forum.kerbalspaceprogram.com/topic/161276-18x-dmagics-basic-mods-basic-orbit-90-basic-deltav-60-11-2-2019/

[gotmachine]

Yeah, I'm aware of that one, but :

• It is obsolete and has multiple issues in current KSP versions.
• It re-implement a side by side solver based on an obsolete version of KER, meaning that users of KER are actually are paying double the performance cost, and that the dv calcs themselves have various issues that were utlimately fixed in KER.
• It just hijack the stock UI but doesn't populate the underlying data structures, meaning that anything relying on the stock dv calcs won't work (might not be too much of an annoyance in practice, but I remember it breaking a few other plugins, not sure which ones).

[JonnyOThan]

Before any action is taken I'd like to understand why this was showing 140ms in the profiler and doesn't seem to be an issue in release. I'd be wary of replacing the stock code with something that could very well be worse. I'll try and get the craft file...

RO disables the stock dv calculator in favor of MJ right? At least there's precedent there.

[siimav]

RO disables the stock dv calculator in favor of MJ right? At least there's precedent there.

Correct.

[gotmachine]

RO disables the stock dv calculator in favor of MJ right?

RP1 does that, but it also disable the whole stock data, readouts and UI in a controlled environment where nothing is supposed to use it.

[gotmachine]

Just did a few profiling tests, on a very large single stage vessel (~500 parts) with 38 engines.
I profiled manually VesselDeltaV.SimulateFlightScene(), on a per call basis (this is running approximately every 15 frames), both in debug enabled install (but no profiler attached) and in a regular, unmodified install.

	engines not staged	engines not running	engines running
Simulate - debug install	0.014 ms	108 ms	148 ms
Frame - debug install	11 ms	122 ms	165 ms
Simulate - vanilla install	0.004 ms	35 ms	53 ms
Frame - vanilla install	9 ms	44 ms	63 ms

So, yeah, a testament to how skewed the profiler results can be...

VesselDeltaV.RunCalculations() only runs when the vessel is modified, and is a coroutine spreading some calcs over a few frames, 3 in my case but this depends on the amount of stages :

	frame 1	frame 2	frame 3
debug	16 ms	112 ms	2.5 ms
vanilla	6 ms	46 ms	1.2 ms

38 engines running on a 500 part vessel mighty start to be an extreme example, but no that much, and regardless of the method used to profile, SimulateFlightScene() is definitely causing a massive frame time instability.

[gotmachine]

Deep profiling results with the unity profiler (sorry, can't share that with my crappy internet, the file is 4 GB...)

I've verified that DeltaVStageInfo.CheckTimeStep() is indeed eating up half the call time even in a non-debug install, mostly due to the awful implementation of PartSet.ContainsPart()
Ultimately this boils down to how inefficient the underlying PartSet stuff is, not so sure there is much to be done...
Well, maybe there is something to be done, but I don't have the courage to look at it in depth...
IDK, it seems the code taking half the call time in CheckTimeStep() :

		for (int k = 0; k < resourcePartsToStageNext.Count; k++)
		{
			if (enginesStillActive[j].partInfo.part.simulationCrossfeedPartSet.ContainsPart(resourcePartsToStageNext[k].resourcePart.part))
			{
				flag = true;
				break;
			}
		}

is something that shouldn't change unless the vessel is modified, so maybe the result could be cached somewhere...