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

perf: Minor optimizations #343

Merged
merged 10 commits into from
Dec 5, 2024

Conversation

hangy
Copy link
Contributor

@hangy hangy commented Dec 2, 2024

Some minor performance optimizations I stumbled upon while having a look at #341:

  1. FallbackLanguagesCollection
    1. Avoid dictionary lookup for the "default" fallback case
    2. Avoid ContainsKey/[key] lookup by shortcutting with TryGetValue
  2. JsonConverter
    1. Replaced ICollection<string> with List<string> to avoid unnecessary virtual method calls
    2. Use char instead of string
    3. Optimize some string/collection operations with range operator and Span<T>
  3. LocalizationProvider
    1. Use source-generated compiled regex to detect placeholders
    2. Use a token-based approach to replace placeholders to avoid regex allocations

@hangy hangy marked this pull request as draft December 3, 2024 20:05
@stefanolsen
Copy link
Contributor

@hangy Can you also replace new CultureInfo(language) with CultureInfo.GetCultureInfo(language) in FallbackLanguagesCollection.cs, since you are already working on that file?

@hangy
Copy link
Contributor Author

hangy commented Dec 4, 2024

@hangy Can you also replace new CultureInfo(language) with CultureInfo.GetCultureInfo(language) in FallbackLanguagesCollection.cs, since you are already working on that file?

I can't find it there. You're talking about this file, correct?

@hangy hangy marked this pull request as ready for review December 4, 2024 22:49
@stefanolsen
Copy link
Contributor

@hangy My bad. I was referring to this.

@valdisiljuconoks valdisiljuconoks self-assigned this Dec 5, 2024
@valdisiljuconoks valdisiljuconoks added this to the 9.0.0 milestone Dec 5, 2024
@valdisiljuconoks valdisiljuconoks changed the base branch from master to v9 December 5, 2024 09:52
@valdisiljuconoks valdisiljuconoks self-requested a review December 5, 2024 09:52
@valdisiljuconoks
Copy link
Owner

@stefanolsen I'll finish CultureInfo.Get(...) in v9 branch

@valdisiljuconoks valdisiljuconoks merged commit 45ee7b1 into valdisiljuconoks:v9 Dec 5, 2024
1 of 3 checks passed
@hangy hangy deleted the perf-experiments branch December 6, 2024 12:30
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