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

Don't copy mutation branches unecessarily #48094

Merged
merged 1 commit into from
Mar 17, 2021

Conversation

anothersimulacrum
Copy link
Member

@anothersimulacrum anothersimulacrum commented Mar 17, 2021

Summary

Performance "Speed up navigation in traits menu"

Purpose of change

I've never experienced this menu to be terribly slow, but @KorGgenT has often complained about it in the past.
I profiled it and saw that checking if traits conflict via has_same_trait_type was taking a huge percentage of the time.

Describe the solution

My profiling showed that most of the overhead from has_same_trait_type was due to get_mutations_in_type, and most of that due to constructing mutations branches. So, stop taking uneccessary copies, and use references.

Describe alternatives you've considered

This is about a five minute fix, and perf is telling me it's a huge improvement, so I'm happy to leave it here.

Testing

perf record -g -p PID when in the traits menu, scroll around and select a few traits.

Additional context

Before:
image
After:
image

@anothersimulacrum anothersimulacrum added [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) labels Mar 17, 2021
@ZhilkinSerg ZhilkinSerg merged commit 5b4060f into CleverRaven:master Mar 17, 2021
@anothersimulacrum anothersimulacrum deleted the trait-opt branch March 17, 2021 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants