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

Optimize working with filters and extensions in FileDialog, decrease allocations #9599

Merged
merged 5 commits into from
Jan 31, 2025

Conversation

h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Aug 18, 2024

Description

Decreases allocations and improves performance when working with FileDialog and the derived classes.

  • COMDLG_FILTERSPEC is now a readonly struct, it is defined as const in the COM interop.
  • Replaced string overloads for StartsWith, IndexOf etc. with char overload instead of single-character strings.
  • string.CompareOrdinal replaced with string.Equals(a, b, StringComparison.Ordinal)
  • Instead of using string.Split to get Length we count them using MemoryExtensions.Count
  • GetFilterItems will no longer allocate a List<COMDLG_FILTERSPEC> with ToArray() conversion.
  • GetFilterExtensions will no longer allocate a List<string> with ToArray() conversion.
  • Removed obsolete CAS remarks on FileDialog and derived classes.

Using string.Split vs MemoryExtensions.Count for counting

Method findPattern Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original Image(...))|*.* [68] 47.795 ns 0.7218 ns 0.6027 ns 0.0172 672 B 288 B
PR__EDIT Image(...))|*.* [68] 4.336 ns 0.0174 ns 0.0154 ns - 98 B -
Benchmark code
[Benchmark]
[Arguments("Image Files(*.BMP;*.JPG;*.GIF)|*.BMP;*.JPG;*.GIF|All files (*.*)|*.*")]
public  int Original(string updatedFilter)
{
    string[] formats = updatedFilter.Split('|');

    return formats.Length; 
}

[Benchmark]
[Arguments("Image Files(*.BMP;*.JPG;*.GIF)|*.BMP;*.JPG;*.GIF|All files (*.*)|*.*")]
public int PR__EDIT(string updatedFilter)
{
    return findPattern.AsSpan().Count("|");
}

GetFilterItems benchmark

Method filter Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original Image(...))|*.* [68] 80.42 ns 1.255 ns 1.113 ns 0.0277 2,413 B 464 B
PR__EDIT Image(...))|*.* [68] 58.19 ns 0.809 ns 0.757 ns 0.0206 870 B 344 B
Original Imag(...)|*.* [206] 213.0 ns 4.28 ns 5.71 ns 0.0720 2,402 B 1208 B
PR__EDIT Imag(...)|*.* [206] 158.0 ns 3.19 ns 4.97 ns 0.0558 870 B 936 B
Benchmark code
[Benchmark]
[Arguments("Image Files(*.BMP;*.JPG;*.GIF)|*.BMP;*.JPG;*.GIF|All files (*.*)|*.*")]
public COMDLG_FILTERSPEC[] Original(string filter)
{
    var extensions = new List<COMDLG_FILTERSPEC>();

    if (!string.IsNullOrEmpty(filter))
    {
        string[] tokens = filter.Split('|');
        if (0 == tokens.Length % 2)
        {
            for (int i = 1; i < tokens.Length; i += 2)
            {
                 extensions.Add(new COMDLG_FILTERSPEC_OLD() { pszSpec = tokens[i - 1], pszName = tokens[i] });
            }
        }
    }
    return extensions.ToArray();
}

  [Benchmark]
  [Arguments("Image Files(*.BMP;*.JPG;*.GIF)|*.BMP;*.JPG;*.GIF|All files (*.*)|*.*")]
  public COMDLG_FILTERSPEC[] PR__EDIT(string filter)
  {
      COMDLG_FILTERSPEC[] extensions = null;

      if (!string.IsNullOrEmpty(filter))
      {
          string[] tokens = filter.Split('|');
          if (tokens.Length % 2 == 0)
          {
              extensions = new COMDLG_FILTERSPEC[tokens.Length / 2];
              for (int i = 0; i < extensions.Length; i++)
              {
                  extensions[i] = new(tokens[i * 2], tokens[i * 2 + 1]);
              }
          }
      }

      return extensions ?? Array.Empty<COMDLG_FILTERSPEC>();
  }

Customer Impact

Improved performance, decreased allocations.

Regression

No.

Testing

Local build, testing with file dialogs.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners August 18, 2024 19:28
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Aug 18, 2024
harshit7962
harshit7962 previously approved these changes Jan 31, 2025
Copy link
Member

@harshit7962 harshit7962 left a comment

Choose a reason for hiding this comment

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

LGTM, @h3xds1nz, feel free to resolve the conflicts. This can be merged post that.

@h3xds1nz
Copy link
Member Author

@harshit7962 Resolved :)

Copy link
Member

@harshit7962 harshit7962 left a comment

Choose a reason for hiding this comment

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

Re-Approving

@harshit7962 harshit7962 merged commit 59e9cc0 into dotnet:main Jan 31, 2025
8 checks passed
@harshit7962
Copy link
Member

Thank you @h3xds1nz for the contribution.

@h3xds1nz
Copy link
Member Author

@harshit7962 Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants