Skip to content

Use span slice instead of substring in AbbreviatedGeometryParser.ReadNumber #6272

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

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

stephentoub
Copy link
Member

Description

AbbreviatedGeometryParser.ReadNumber is using Substring to pass to Convert.ToDouble the relevant portion of the string to parse. We can instead just pass in a span slice to double.Parse (which Convert.ToDouble delegates to after a null check that's irrelevant here).

Customer Impact

Unnecessary allocations.

Regression

No

Testing

CI

Risk

Minimal

@stephentoub stephentoub requested a review from a team as a code owner March 17, 2022 18:02
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Mar 17, 2022
@ghost ghost requested review from dipeshmsft, singhashish-wpf and SamBent March 17, 2022 18:02
@stephentoub stephentoub force-pushed the parserreadnumbersubstring branch from 3de9b27 to da16382 Compare March 17, 2022 20:03
@stephentoub stephentoub force-pushed the parserreadnumbersubstring branch from da16382 to 861ba11 Compare March 17, 2022 20:47
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 left a comment

Choose a reason for hiding this comment

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

This PR is a duplicate of #5390 but in my opinion the changes in this PR are slightly better since it has less ifdefed code.

We should either close #5390 and keep this PR or close this PR and move the changes from this PR to #5390.

/cc @lindexi

@dipeshmsft dipeshmsft self-assigned this May 5, 2022
@ghost ghost assigned stephentoub May 5, 2022
…edia/ParsersCommon.cs

Co-authored-by: ThomasGoulet73 <51839772+ThomasGoulet73@users.noreply.github.com>
@singhashish-wpf singhashish-wpf merged commit f1cfb42 into dotnet:main Jul 21, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants