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

Performance: Ships with large numbers of engines are slow because of interleaved transform updates/raycasts #216

Closed
JonnyOThan opened this issue Mar 13, 2024 · 5 comments
Labels
kspPerformance Possible performance improvement in KSP

Comments

@JonnyOThan
Copy link
Contributor

Engines have to do a raycast to see if the thrust is obstructed. In the stock code this happens in the engine module's FixedUpdate. The problem is that this is basically interleaved with transform updates, and >90% of the cost of the raycasts is in syncing collider transforms. If the engine raycasts were batched together, the sync would only need to happen once and it should be much faster.

image

@JonnyOThan JonnyOThan added the kspPerformance Possible performance improvement in KSP label Mar 13, 2024
@siimav
Copy link
Contributor

siimav commented Mar 13, 2024

Yup, that's what we already solved in SolverEngines.
KSP-RO/SolverEngines@b3b7a95

@JonnyOThan
Copy link
Contributor Author

Awesome....I think we could incorporate some of that code in KSPCF since it's LGPL?

Also note that Solar panels have a similar problem.

@gotmachine
Copy link
Contributor

gotmachine commented Mar 14, 2024

Transform sync only happens if there was actually some changed transforms in between two separate engines fixedupdates, which will indeed be the case when gimbals are actively moving.

Batching the raycasts and engine damage execution at a deferred point of execution would indeed be ideal, but we should strictly limit that to stock modules, as we can't predict the behavior of modded engines (ie, SolverEngines...). Handling other modules that also perform raycasts (solar panels, radiators) would be more difficult as the core functionality of those modules is dependant on the raycasts results, so deffering execution would have even more consequences (and in the case of solar panels, there are definitely a lot of plugins either subclassing them or relying on the stock behavior).

Maybe a less intrusive solution would be to simply ensure a call to Physics.SyncTransforms is performed prior to the first raycast of any of those modules, then to disable Physics.autoSyncTransforms when the raycasts are performed.

The risk of doing that would be that if a partmodule is changing a position/rotation in between, the raycast results won't account for that change, but I fail to see any case where this could be a practical issue. There aren't many reasons to move transforms from a partmodule, and the few I can think of are small incremental moves that won't matter given the level of precision the raycasts are used for. Even if we have a partmodule moving the whole vessel, which I would expect to be a rare event, the raycasts will be accurate as far as the relative position of parts are involved. This could only matter for raycasting against the environment/terrain, and I fail to see a use case for continuously teleporting the vessel when in the vicinity of something else.

gotmachine added a commit that referenced this issue Mar 25, 2024
Optimization of raycasts in ModuleEngines.EngineExhaustDamage() and ModuleDeployableSolarPanel.CalculateTrackingLOS() :
- Only synchronize transforms on the first raycast from any module, mainly relevant when something else is moving transforms in between calls, which is often the case for active engines with gimbals.
- Cached ScaledSpace raycast results for solar panels : call time is divided by between 4 (when blocked by a scaled space object) and 2 (when not blocked)
gotmachine added a commit that referenced this issue Apr 4, 2024
…tion

Optimization of raycasts in ModuleEngines.EngineExhaustDamage() and ModuleDeployableSolarPanel.CalculateTrackingLOS() :
- Only synchronize transforms on the first raycast from any module, mainly relevant when something else is moving transforms in between calls, which is often the case for active engines with gimbals.
- Cached ScaledSpace raycast results for solar panels : call time is divided by between 4 (when blocked by a scaled space object) and 2 (when not blocked)
@gotmachine
Copy link
Contributor

Fix implemented in #220

@gotmachine
Copy link
Contributor

Released in 1.35.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kspPerformance Possible performance improvement in KSP
Development

No branches or pull requests

3 participants