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

Optimize str_format(…, …, "%d", …) using templates #8363

Merged
merged 2 commits into from
May 22, 2024

Conversation

heinrich5991
Copy link
Member

This would make the function str_from_int unnecessary, at least user-facing.

Advantage: User doesn't have to care about str_from_int/str_format distinction.

Disadvantage: We're adding some template programming to system.h, potentially slowing all compilation.

Checklist

  • Tested the change ingame
  • Provided screenshots if it is a visual change
  • Tested in combination with possibly related configuration options
  • Written a unit test (especially base/) or added coverage to integration test
  • Considered possible null pointers and out of bounds array indexing
  • Changed no physics that affect existing maps
  • Tested the change with ASan+UBSan or valgrind's memcheck (optional)

@heinrich5991
Copy link
Member Author

@Learath2 @Robyt3 Do we want something like this?

@Learath2
Copy link
Member

I like it. I'm curious if this gets optimized out properly to a direct call for compile time constant format strings

@heinrich5991
Copy link
Member Author

I checked with all major compilers on Godbolt and this is what I ended up with. It gets optimized out properly for compile-time constant format strings.

@Chairn
Copy link
Contributor

Chairn commented May 14, 2024

This looks like over optimization. What happens if the string is " %d" or "%d\n" or even any other format like %lld or %u ?

@heinrich5991
Copy link
Member Author

This looks like over optimization. What happens if the string is " %d" or "%d\n" or even any other format like %lld or %u ?

It doesn't work in this case. What I'd like to avoid is humans having to remember to call str_from_int. This function was introduced for measurable performance impact (#7056):

This increases FPS in the editor by ~25% when many numbers are rendered for switch/tele/speedup/tune layers or with "Show Info" being enabled.

@Robyt3
Copy link
Member

Robyt3 commented May 14, 2024

Looks good. I prefer this over having a separate str_from_int.

@heinrich5991 heinrich5991 marked this pull request as ready for review May 14, 2024 19:15
@heinrich5991 heinrich5991 force-pushed the pr_ddnet_str_from_int branch 3 times, most recently from a1f2e4f to 2e85310 Compare May 22, 2024 11:47
This would make the function `str_from_int` unnecessary, at least
user-facing.

Advantage: User doesn't have to care about `str_from_int`/`str_format`
distinction.

Disadvantage: We're adding some template programming to `system.h`,
potentially slowing all compilation.
@Robyt3 Robyt3 added this pull request to the merge queue May 22, 2024
Merged via the queue into ddnet:master with commit 68d474c May 22, 2024
16 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.

None yet

5 participants