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 static readonly arrays for separators #4936

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gpetrou
Copy link

@gpetrou gpetrou commented Dec 23, 2024

Pull Request Template

Description

Avoid allocating new arrays for separators when splitting strings.
Related code analysis warning https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1861.

Type of change

Small performance-related change.

@gpetrou gpetrou force-pushed the StaticReadonlySeparators branch from 74eede4 to bbda2ec Compare December 23, 2024 06:50
@gpetrou gpetrou marked this pull request as ready for review December 23, 2024 06:51
@gpetrou
Copy link
Author

gpetrou commented Dec 23, 2024

@microsoft-github-policy-service agree

Pilchie
Pilchie previously approved these changes Dec 29, 2024
@@ -113,6 +113,10 @@ internal partial class DocumentClient : IDisposable, IAuthorizationTokenProvider
private const bool DefaultEnableCpuMonitor = true;
private const string DefaultInitTaskKey = "InitTaskKey";

private static readonly char[] databaseLinkSeparators = new char[] { '/' };
private static readonly char[] sourceDocumentCollectionLinkSeparators = new char[] { '/' };
Copy link
Member

Choose a reason for hiding this comment

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

Both Line 116 and 117 can be same which is path separator.

@kirankumarkolli
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@gpetrou gpetrou dismissed stale reviews from Pilchie and kirankumarkolli via 2cb3822 December 31, 2024 05:34
@gpetrou gpetrou force-pushed the StaticReadonlySeparators branch from 6666c20 to 2cb3822 Compare December 31, 2024 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants