-
Notifications
You must be signed in to change notification settings - Fork 351
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 ReadOnlySpan<char> to replace the 'new string(...)' to reduce memory allocation #3077
Conversation
…e the memory allocation during parsing. Be noted, the ReadOnlySpan<char> doesn't override the operator == and != for string. Remember to use 'Equals(...)' to replace them. Anytime to call 'ToString()' on ReadOnlySpan<char> will do the 'new string(...)', so be noted, not to call it mulitple times
@xuzhg could you share the benchmarks that you used for the perf tests, did you also try to run the existing UriParser benchmarks before and after these changes? |
src/Microsoft.OData.Core/UriParser/Parsers/CountSegmentParser.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.OData.Core/UriParser/Parsers/SelectExpandOptionParser.cs
Outdated
Show resolved
Hide resolved
I create a branch to hold the perf test at: https://github.com/OData/odata.net/tree/ReadOnlySpanBenchMark This PR is just a starting point. We will move to UriParser and others in the comming step. |
@habbes Resolved the comment. Please take a look again. I will keep moving to next step. It could be 'UriParser'. |
Welcome any new comments/reviews. I will triage them together, but I have to merge it now and move my work forward. |
Use ExpressionLexer to do ReadOnlySpan investigation and seek feebacks to move forward to all other ODL parts.
Be noted,
It outputs
Benchmark:
See the allocated, using 'ReadOnlySpan' is 1/3 memory allocation comparing to the 'new string(...)'.
Codes
Issues
This pull request fixes #xxx.
Description
Briefly describe the changes of this pull request.
Checklist (Uncheck if it is not completed)
Additional work necessary
If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.