Skip to content

Replace IsAssignableFrom with is in converters#5933

Merged
singhashish-wpf merged 1 commit intodotnet:mainfrom
ThomasGoulet73:replace-isassignablefrom-with-is-in-converters
Mar 30, 2022
Merged

Replace IsAssignableFrom with is in converters#5933
singhashish-wpf merged 1 commit intodotnet:mainfrom
ThomasGoulet73:replace-isassignablefrom-with-is-in-converters

Conversation

@ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Jan 11, 2022

Description

Replace IsAssignableFrom with is in converters. It is faster, has smaller IL and smaller JIT code.

I used this benchmark to ensure that it is in fact faster with MenuScrollingVisibilityConverter as an example.

Here is the result:

| Method |    values | parameter |     Mean |    Error |   StdDev | Ratio |  Gen 0 | Code Size | Allocated |
|------- |---------- |---------- |---------:|---------:|---------:|------:|-------:|----------:|----------:|
|    Old | Object[4] |         0 | 41.75 ns | 0.623 ns | 0.552 ns |  1.00 | 0.0076 |   1,037 B |      24 B |
|    New | Object[4] |         0 | 14.18 ns | 0.175 ns | 0.146 ns |  0.34 | 0.0076 |     936 B |      24 B |

Customer Impact

None.

Regression

No.

Testing

Tested a few converters locally.

Risk

None.

@ThomasGoulet73 ThomasGoulet73 requested a review from a team as a code owner January 11, 2022 05:17
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jan 11, 2022
@ghost ghost requested review from SamBent, fabiant3 and ryalanms January 11, 2022 05:17
@singhashish-wpf singhashish-wpf merged commit bdfe29a into dotnet:main Mar 30, 2022
@ThomasGoulet73 ThomasGoulet73 deleted the replace-isassignablefrom-with-is-in-converters branch March 30, 2022 17:10
@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2022
@ghost ghost assigned ThomasGoulet73 May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fabiant3 fabiant3 Awaiting requested review from fabiant3

@ryalanms ryalanms Awaiting requested review from ryalanms

@SamBent SamBent Awaiting requested review from SamBent

1 more reviewer

@singhashish-wpf singhashish-wpf singhashish-wpf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ThomasGoulet73 ThomasGoulet73

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ThomasGoulet73 @singhashish-wpf @dipeshmsft