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

First pass at performance testing #1198

Merged
merged 112 commits into from
Oct 16, 2023
Merged

First pass at performance testing #1198

merged 112 commits into from
Oct 16, 2023

Conversation

csciguy8
Copy link
Contributor

@csciguy8 csciguy8 commented Aug 29, 2023

Add initial performance testing for Unreal. Also adds profiling guides for MSVS CPU usage and Unreal Insights.

Similar to unit tests, performance tests can be run using the Unreal Automation System. The goal is to create repeatable scenarios used for assessing various performance criteria. They are programmatically generated, can pass / fail based on whatever criteria we chose, and can eventually run on continuous integration (CI).

This PR adds a 2 simple load tests, each using different locations, and will measure the time it takes for all tilesets to load fully. Results are output to the console (log). If you are running from the editor when the test has finished, you will see the scene that was generated and all tiles frozen in place.

Comments welcome. This PR was intentionally left simple to serve as discussion for future work in this area.

Relates to cesium-native issue 698

Load Test Capture (for github)

@cesium-concierge
Copy link

Thanks for the pull request @csciguy8!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

When PIE ends, it seems to clear all tiles. Before we froze the state of what the test loaded, which was useful. But it's not all that useful to just see nothing when the test ends. Let it stream it all back in.

Also, move play session param to context so it can be configured if needed
Melbourne 'after' test is temporarily commented out
Copy link
Member

@kring kring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a couple of last small details here.

Source/CesiumRuntime/Private/Cesium3DTileset.cpp Outdated Show resolved Hide resolved
@csciguy8
Copy link
Contributor Author

Thanks for the the review @kring . Ready for another look.

@@ -2077,25 +2077,25 @@ void ACesium3DTileset::Tick(float DeltaTime) {
CreateViewStateFromViewParameters(camera, unrealWorldToCesiumTileset));
}

Cesium3DTilesSelection::ViewUpdateResult result;
const Cesium3DTilesSelection::ViewUpdateResult* result;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but it would be better to name this pResult. https://github.com/CesiumGS/cesium-native/blob/main/doc/style-guide.md#-naming

Sometimes we use Unreal's naming conventions instead (we should be more consistent about this), in which case the proper name would be Result.

I'll make this change and then merge this in order to avoid another cycle just for trivial changes.

@kring
Copy link
Member

kring commented Oct 16, 2023

Thanks @csciguy8! 🎉

@kring kring merged commit 20b6e63 into ue5-main Oct 16, 2023
10 of 16 checks passed
@kring kring deleted the simple-performance-test branch October 16, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants