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

Use code generated or compiled regular expressions where possible #8141

Merged
merged 1 commit into from
Nov 18, 2022

Conversation

SukharevAndrey
Copy link
Contributor

@SukharevAndrey SukharevAndrey commented Nov 12, 2022

This PR replaces all regular expression usages in the project with source generated or at least dynamically compiled ones. It should increase performance a bit, especially in code generator which is now using source generators, isn't it? Also projects using Orleans will have more room for their regexps' cache because global Regex cache's size is quite small (15) and Orleans will not use it now.

As it is my first contribution to Orleans, I still have questions:

  • I have doubts about using source generated regexps due to the fact that classes containing regexps need to be made partial. Is this OK? Should I change that usages to ordinary regexps with Compiled option?
  • I have some more low-hanging fruits in mind to fix. Should I create an issue for that (including this PR)?

I'm planning to do more meaningful contributions (such as analyzers) so any feedback is appreciated. Thanks.

Microsoft Reviewers: Open in CodeFlow

@dnfadmin
Copy link

dnfadmin commented Nov 12, 2022

CLA assistant check
All CLA requirements met.

@SukharevAndrey SukharevAndrey requested review from ReubenBond and IEvangelist and removed request for ReubenBond and IEvangelist November 15, 2022 11:19
@ReubenBond ReubenBond merged commit b471cfe into dotnet:main Nov 18, 2022
@SukharevAndrey SukharevAndrey deleted the optimize/regexp-usages branch November 19, 2022 21:09
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants