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

Use Array.IndexOf to speed up Shroud.Tick. #21185

Merged
merged 3 commits into from Nov 4, 2023

Conversation

RoosterDragon
Copy link
Member

As the touched cell layer uses Boolean values, Array.IndexOf is able to use a fast vectorised search. Most values in the array are false, so the search is able to significantly improve the performance of finding the next true value in the array.


Review with ignore whitespace for a cleaner diff.

Running this replay at max speed on commit 5157bc3 gives the following performance improvement. ra-2023-11-04T105726Z.zip

Before

  • Shroud.Tick 14.3%
    • OnShroudChanged 2.5%

After

  • Shroud.Tick 6.5%
    • UpdateCell 4.0%
      • OnShroudChanged 2.9%
    • IndexOf 1.8%

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

Using /all a second time now blinds you. Even when you have explored map option enabled

@RoosterDragon
Copy link
Member Author

RoosterDragon commented Nov 4, 2023

Using /all a second time now blinds you. Even when you have explored map option enabled

Looks like an existing issue on bleed, as using the /all command simulates using the "Clear Shroud" and "Reset Shroud" buttons. Have added a commit that removes those buttons from the /all command which fixes the problem.

As the `touched` cell layer uses Boolean values, Array.IndexOf is able to use a fast vectorised search. Most values in the array are false, so the search is able to significantly improve the performance of finding the next true value in the array.
Disabling the shroud is sufficient to allow seeing the map. This fixes a game with the "Explored Map" option enabled. Previously using the `/all` command twice to toggle it on and off again would also reset the shroud, causing the map to no longer be explored. Now, using it twice will cause the map to remain explored, as intended when the "Explored Map" option is enabled.
@PunkPun
Copy link
Member

PunkPun commented Nov 4, 2023

Oh hmm, it looks like this issue in in the current release as well

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

@PunkPun PunkPun merged commit e83e580 into OpenRA:bleed Nov 4, 2023
3 checks passed
@RoosterDragon RoosterDragon deleted the shroud-perf branch November 4, 2023 16:46
@PunkPun
Copy link
Member

PunkPun commented Nov 4, 2023

Changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants