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

Remove the regex from data protection #28114

Closed
wants to merge 2 commits into from
Closed

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented Nov 24, 2020

  • This also affects startup time since the regex is compiled

cc @stephentoub

@ghost ghost added the area-dataprotection Includes: DataProtection label Nov 24, 2020
if (versionEndIndex == -1)
{
// No end index?
return forwardedTypeName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be:

return forwardedTypeName.Substring(0, versionStartIndex);

? Otherwise it could contain a Version=something but nothing after it?

@stephentoub
Copy link
Member

stephentoub commented Nov 24, 2020

This also affects startup time since the regex is compiled

For reference, I pinged David about this as this regex was being compiled as part of a default MVC app startup and taking ~8% of the time.

Related to dotnet/runtime#44598

@davidfowl
Copy link
Member Author

The tests are a bit more complex than removing a single version from a type name it seems. The current implementation handles generics (which I'm dubious is even a scenario that happens).

@stephentoub
Copy link
Member

which I'm dubious is even a scenario that happens

What is the scenario here? With the default MVC template, I see this code kicking in to forward several types; why should anything new require forwarding?

@davidfowl
Copy link
Member Author

Will revisit. @HaoK if you're looking at data protection we should look at this.

@davidfowl davidfowl closed this Feb 23, 2021
@HaoK
Copy link
Member

HaoK commented Feb 23, 2021

Sure, I can roll this into the span work, linking to #30013

@davidfowl davidfowl deleted the davidfowl/remove-regex branch July 25, 2021 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dataprotection Includes: DataProtection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants