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 ReadOnlySpan<char> to replace 'new string(...)' #3094

Closed
xuzhg opened this issue Oct 17, 2024 · 0 comments · Fixed by #3097
Closed

Use ReadOnlySpan<char> to replace 'new string(...)' #3094

xuzhg opened this issue Oct 17, 2024 · 0 comments · Fixed by #3097

Comments

@xuzhg
Copy link
Member

xuzhg commented Oct 17, 2024

Short summary (3-5 sentences) describing the issue.

Assemblies affected

Which assemblies and versions are known to be affected e.g. OData .Net lib 7.x

ODL has lot of string parsing scenarios, for example:

1) OData Uri parsing
2) OData query option parsing
3) OData json payload parsing

The typical parsing process is:

  1. Read character one by one
  2. 'Create a new substring' every time if it could be a 'token'
  3. Create the corresponding token instance using the substring
  4. Do the further process

For example:

"abc(2014-09-19T12:13:14+00:00)/3258.678765765489753678965390/SRID=1234(POINT(1020))/Function(foo=@x,bar=1,baz=@y)"

If do a lexer, we can get the following tokens:

Kind: Identifier: Text: abc
Kind: OpenParen: Text: (
Kind: DateTimeOffsetLiteral: Text: 2014-09-19T12:13:14+00:00
Kind: CloseParen: Text: )
Kind: Slash: Text: /
Kind: DecimalLiteral: Text: 3258.678765765489753678965390
Kind: Slash: Text: /
Kind: Identifier: Text: SRID
Kind: Equal: Text: =
Kind: IntegerLiteral: Text: 1234
Kind: OpenParen: Text: (
Kind: Identifier: Text: POINT
Kind: OpenParen: Text: (
Kind: IntegerLiteral: Text: 10
Kind: IntegerLiteral: Text: 20
Kind: CloseParen: Text: )
Kind: CloseParen: Text: )
Kind: Slash: Text: /
Kind: Identifier: Text: Function
Kind: OpenParen: Text: (
Kind: Identifier: Text: foo
Kind: Equal: Text: =
Kind: Identifier: Text: @x
Kind: Comma: Text: ,
Kind: Identifier: Text: bar
Kind: Equal: Text: =
Kind: IntegerLiteral: Text: 1
Kind: Comma: Text: ,
Kind: Identifier: Text: baz
Kind: Equal: Text: =
Kind: Identifier: Text: @y
Kind: CloseParen: Text: )

As an example:
1) We have to create a string for literal l'2014-09-19T12:13:14+00:00'
2) Test it using the created substring
3) Create the DateTimeOffset instance using that string
4) The created substring goes into the GC

Moreover, most of time, we don't need the 'substring', for example, "(", ")", etc, They are special but no other meaning, just a label. We don't need to create a substring for all of them.

In a summary, it seems we don't need to create a 'new string'. Because the literal is there (in the original raw string), we just need to retrieve it when we do need it. And most importantly, if we just test it (to see whether it's a valid DateTimeOffset literal), we can just test the PART of original raw string, don't need to create a new substring.

Expected result

What would happen if there wasn't a bug.

Actual result

What is actually happening.

Additional detail

Optional, details of the root cause if known. Delete this section if you have no additional details to add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants