Add an AllocateString overload for InlineSString& #64
Conversation
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
Calling AllocateString passing in an InlineSSString will compile, but AllocateString overload that is calling takes an SString. This ends up calling the SString(WCHAR*) conversion constructor, which means we pay for another strlen & allocation. To prevent us from accidentally hitting this again, I've added local AllocateString that will take an InlineString by reference.
scott-ferguson-unity
requested review from
joncham,
brian-taylor-unity and
UnityAlex
joncham
approved these changes
Apr 26, 2022
brian-taylor-unity
approved these changes
Apr 26, 2022
UnityAlex
approved these changes
Apr 26, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Calling AllocateString passing in an InlineSSString
will compile, but AllocateString overload that is
calling takes an SString. This ends up calling the
SString(WCHAR*) conversion constructor, which means
we pay for another strlen & allocation.
To prevent us from accidentally hitting this again,
I've added local AllocateString that will take an
InlineString by reference.
On may machine this drops 1M
mono_new_string_wrappercalls from 185ms to 109ms.