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

chore: Use vector instead of forward_list for dpGrid for ~50% physics loop speed improvement #1447

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

EmosewaMC
Copy link
Collaborator

also use contains instead of find != end on an associative container since we are in c++20 now :)

Metrics below were captured after spawning 200 Stromling Admirals (6789) and leading them on a path through Ninjago Monastary which consisted of going from the Fire Dojo all the way to the launchpad. The frames before tests however only went to the launchpad as just making half the trek was sufficient data to show one version is faster and extra data was just driving the point home further. For an estimation, double all numbers in frames before.

Testing was conducted by also giving all admirals an aggro radius, soft tether radius and hard tether radius of 100'000 and the following code block immediately after the Measurement for Physics was ended if (zoneID == 2000) LOG("physics is %llu", Metrics::GetMetric(MetricVariable::Physics)->average);

searching for [6-9]{1}[0-9]{5} (6 digit physics processing times that are > 600000 and < 999999

8422 frames before
4688 frames after

searching for [7-9]{1}[0-9]{5} (6 digit physics processing times that are > 700000 and < 999999

6043 frames before
0 frames after

searching for [8-9]{1}[0-9]{5} (6 digit physics processing times that are > 800000 and < 999999

3833 frames before
0 frames after

searching for [9]{1}[0-9]{5} (6 digit physics processing times that are > 900000 and < 999999

3833 frames before
0 frames after

Easily, the newer vector approach is faster when iterating the vectors in the update loop, however we need to take into account that Move Delete and Add are not counted in the above metrics. The same test case above was done except this time the used metric was the full frame time since several places could cause a Move Delete or Add of the physics grid. The following log was placed immediately following the measurement end of GameLoop if (zoneID == 2000) LOG("gameloop is %llu", Metrics::GetMetric(MetricVariable::GameLoop)->average);. This test was also only done going from the Fire Dojo to the launch pad.

searching for [2]{1}[0-9]{1}[0-9]{6}\n (7 digit gameloop processing times that are > 20000000 and < 29999999

3755 frames before
3910 frames after

searching for 2]{1}[5-9]{1}[0-9]{6}\n (7 digit gameloop processing times that are > 25000000 and < 29999999

2967 frames before
3022 frames after

searching for [2]{1}[7-9]{1}[0-9]{6}\n (7 digit gameloop processing times that are > 27000000 and < 29999999

296 frames before
239 frames after -- Faster in this case! Could be many variables

We can see here there is a difference in the two methods, the newer one being slower on average by a range of 5% on the lower end, but as the container scales up from the moving between cells and because the standard for a vector.push_back runtime is an amortized constant, we know that the number of elements will grow geometrically (in chunks of 32, 64 etc.) so even if a push_back causes a re-allocation, we only pay the copy cost a lot at the lower capacities, but over time the cost of insertion levels out with the usability of a list.

I have also tested that I can be smashed by PhantomPhysics objects (lava lake in Fire Dojo and also verification that Add is working), that admirals are able to follow me all the way to the ninjago initial rocket spawn (this would indicate if Move is not working if they could not all make it that far) as well as verifying that after killing the 200 admirals, there are not 200 more null entities in the grid (this would indicate Delete was not deleting the right elements from the grid)

also swap contains in for find != end on an associative container since we are in c++20 now :)
@EmosewaMC
Copy link
Collaborator Author

I tried my best to get some good numbers here. If there are cases I missed please let me know.

@jadebenn
Copy link
Collaborator

jadebenn commented Feb 4, 2024

Ah, the MacOS nodiscard gremlins strike again. 😅

Copy link
Member

@DarwinAnim8or DarwinAnim8or left a comment

Choose a reason for hiding this comment

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

From what I know, the std::clamp returns the value, as macos is failing builds on it.

// Clamp values to the range [0, NUM_CELLS - 1]
oldCellX = std::clamp(oldCellX, 0, NUM_CELLS - 1);

should be correct

@DarwinAnim8or DarwinAnim8or merged commit 24f94ed into main Feb 8, 2024
4 checks passed
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.

4 participants