Avoid string allocation/free traffic in assert_in_bounds() - #1433
Open
ishani wants to merge 1 commit into
Open
ishani wants to merge 1 commit into
ishani wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
…eck fails so that the code is not allocating, concatenating and freeing strings that are never used in regular operation - this can produce a severe performance penalty (cherry picked from commit 51f5871)
Copy link
Copy Markdown
Contributor
|
I recommend viewing this with whitespace ignored, as it is then a single line move instead of multiple lines moved. |
Sorry, something went wrong.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move the construction of the OOB violation string to after the OOB check fails so that the code is not allocating, concatenating and freeing strings that are never used in regular operation. The change simply shunts the construction of the msg string inside the bounds check, given that the msg result is only ever used when the condition fails.
Discovered this by accident when an RNG class I had ported to cpp2 was reading and writing to a small fixed-size array and the RNG was used very frequently in an inner loop.
Profiling the app (-O3 build, clang 19) showed considerable work being done building and discarding strings for every access of the std::array member. Although that could be mitigated by switching to unchecked_subscript, avoiding this string build does not change the operation of the check but does remove the penalty.