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

[controls] improve performance of {Binding} #14830

Merged
merged 1 commit into from
May 2, 2023

Conversation

jonathanpeppers
Copy link
Member

I wrote a benchmark for Binding, such as:

[Benchmark]
public void BindName()
{
    var binding = new Binding("Name", source: Source);
    Target.SetBinding(MyObject.NameProperty, binding);
}

Attaching dotnet-trace, I found ~53% of the time was spent in MethodInfo.GetParameters():

core.benchmarks!Microsoft.Maui.Benchmarks.BindingBenchmarker.BindName()
...
microsoft.maui.controls!Microsoft.Maui.Controls.BindingExpression.SetupPart()
System.Private.CoreLib.il!System.Reflection.RuntimeMethodInfo.GetParameters()

Reviewing the code, it was basically calling:

System.Reflection.PropertyInfo.SetMethod.GetParameters()[0].ParameterType;

We can just call PropertyInfo.PropertyType instead, to avoid allocating a ParameterInfo[] and indexing into it.

The results of this change in my benchmarks:

Method Mean Error StdDev Gen0 Gen1 Allocated
--BindName 18.82 us 0.336 us 0.471 us 1.2817 1.2512 10.55 KB
++BindName 18.80 us 0.371 us 0.555 us 1.2512 1.2207 10.23 KB
--BindChild 27.47 us 0.542 us 0.827 us 2.0142 1.9836 16.56 KB
++BindChild 26.71 us 0.516 us 0.652 us 1.9226 1.8921 15.94 KB
--BindChildIndexer 58.39 us 1.113 us 1.143 us 3.1738 3.1128 26.17 KB
++BindChildIndexer 58.00 us 1.055 us 1.295 us 3.1128 3.0518 25.47 KB

-- Before
++ After

This should help scrolling performance such as in #12130, where bindings are updating while scrolling.

I wrote a benchmark for `Binding`, such as:

    [Benchmark]
    public void BindName()
    {
        var binding = new Binding("Name", source: Source);
        Target.SetBinding(MyObject.NameProperty, binding);
    }

Attaching `dotnet-trace`, I found ~53% of the time was spent in
`MethodInfo.GetParameters()`:

    core.benchmarks!Microsoft.Maui.Benchmarks.BindingBenchmarker.BindName()
    ...
    microsoft.maui.controls!Microsoft.Maui.Controls.BindingExpression.SetupPart()
    System.Private.CoreLib.il!System.Reflection.RuntimeMethodInfo.GetParameters()

Reviewing the code, it was basically calling:

    System.Reflection.PropertyInfo.SetMethod.GetParameters()[0].ParameterType;

We can just call `PropertyInfo.PropertyType` instead, to avoid
allocating a `ParameterInfo[]` and indexing into it.

The results of this change in my benchmarks:

|             Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
|------------------- |---------:|---------:|---------:|-------:|-------:|----------:|
|         --BindName | 18.82 us | 0.336 us | 0.471 us | 1.2817 | 1.2512 |  10.55 KB |
|         ++BindName | 18.80 us | 0.371 us | 0.555 us | 1.2512 | 1.2207 |  10.23 KB |
|        --BindChild | 27.47 us | 0.542 us | 0.827 us | 2.0142 | 1.9836 |  16.56 KB |
|        ++BindChild | 26.71 us | 0.516 us | 0.652 us | 1.9226 | 1.8921 |  15.94 KB |
| --BindChildIndexer | 58.39 us | 1.113 us | 1.143 us | 3.1738 | 3.1128 |  26.17 KB |
| ++BindChildIndexer | 58.00 us | 1.055 us | 1.295 us | 3.1128 | 3.0518 |  25.47 KB |

-- Before
++ After

This should help scrolling performance such as in dotnet#12130, where bindings
are updating while scrolling.
@jonathanpeppers jonathanpeppers added the legacy-area-perf Startup / Runtime performance label Apr 28, 2023
@jonathanpeppers jonathanpeppers marked this pull request as ready for review April 28, 2023 17:33
@jonathanpeppers jonathanpeppers merged commit 85625dd into dotnet:main May 2, 2023
@jonathanpeppers jonathanpeppers deleted the Binding-GetParameters branch May 2, 2023 14:37
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2023
@Eilon Eilon added the t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) label May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.5.8529 Look for this fix in 8.0.0-preview.5.8529! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
fixed-in-8.0.0-preview.5.8529 Look for this fix in 8.0.0-preview.5.8529! legacy-area-perf Startup / Runtime performance t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants