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

DictionarySource Performance #186

Closed
karljj1 opened this issue Aug 10, 2021 · 2 comments
Closed

DictionarySource Performance #186

karljj1 opened this issue Aug 10, 2021 · 2 comments

Comments

@karljj1
Copy link
Collaborator

karljj1 commented Aug 10, 2021

Hey.

I noticed that the DictionarySource is iterating through all the keys to find a match, this is probably because of the case sensitivity option. I wonder if it would be possible to use the TryGetValue method when the Dictionary is using the same EqualityComparer?

Something like this?

if (current is Dictionary<string, object> dict && dict.Comparer == selectorInfo.FormatDetails.Settings.GetCaseSensitivityComparer())
{
    if (dict.TryGetValue(selector, out var result))
    {
        selectorInfo.Result = result;
        return true;
    }
}
@axunonb
Copy link
Member

axunonb commented Aug 11, 2021

Hi
That's right: it's because of case-sensitivity. The case-sensitivity setting of SmartFormat works, no matter how the dictionary instance is created. It could e.g. be new Dictionary<string, object>(20, StringComparer.OrdinalIgnoreCase).
We could of course say, that the comparison setting is fully up to the caller, and the SmartFormat setting will not apply.
Is that what you have in mind?
Did you already compare performance of TryGetValue against the current implementation?

axunonb added a commit that referenced this issue Aug 27, 2021
#### Target frameworks

* Changed `netstandard2.0` to `netstandard2.1`.
* `net461` support unchanged.
* Added package `System.Memory`

#### Remove repetitive substring allocations

Connected modifications:

* Added method `Write(ReadOnlySpan<char> text)` to `IFormattingInfo`
* Generated substrings are cached in classes `Format`, `FormatItem`, `LiteralText`, `Placeholder` and `Selector`.
* Evaluating escaped characters for `Placeholder.FormatterOptions` and `LiteralText` work without heap memory allocation.

#### Alignment operator inheritance is optimized

* Alignment implementation introduced with PR [#174](#174) is modified for better performance
* Added method `Placeholder.AddSelector`
* `Placeholder.Selectors` is now internal. Selectors are accessible with `IReadOnlyList<Selector> Placeholder.GetSelectors()`.

#### DictionarySource performance improved

* Implemented suggestion in issue [#186](#186) for better speed and less GC pressure.
* Side effect: We're using the `CaseSensitivityType` of the dictionary for getting the value for a key. `Settings.GetCaseSensitivityComparison()` will not be applied.
@axunonb
Copy link
Member

axunonb commented Aug 30, 2021

Resolved with PR #189, part of v3.0.0-alpha.1

@axunonb axunonb closed this as completed Aug 30, 2021
axunonb added a commit to axunonb/SmartFormat that referenced this issue Mar 10, 2022
#### Target frameworks

* Changed `netstandard2.0` to `netstandard2.1`.
* `net461` support unchanged.
* Added package `System.Memory`

#### Remove repetitive substring allocations

Connected modifications:

* Added method `Write(ReadOnlySpan<char> text)` to `IFormattingInfo`
* Generated substrings are cached in classes `Format`, `FormatItem`, `LiteralText`, `Placeholder` and `Selector`.
* Evaluating escaped characters for `Placeholder.FormatterOptions` and `LiteralText` work without heap memory allocation.

#### Alignment operator inheritance is optimized

* Alignment implementation introduced with PR [axuno#174](axuno#174) is modified for better performance
* Added method `Placeholder.AddSelector`
* `Placeholder.Selectors` is now internal. Selectors are accessible with `IReadOnlyList<Selector> Placeholder.GetSelectors()`.

#### DictionarySource performance improved

* Implemented suggestion in issue [axuno#186](axuno#186) for better speed and less GC pressure.
* Side effect: We're using the `CaseSensitivityType` of the dictionary for getting the value for a key. `Settings.GetCaseSensitivityComparison()` will not be applied.
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

No branches or pull requests

2 participants