Skip to content

Use Castle to generate delegate proxies#537

Merged
dtchepak merged 1 commit intomasterfrom
use-castle-to-generate-delegate-proxy
Apr 13, 2019
Merged

Use Castle to generate delegate proxies#537
dtchepak merged 1 commit intomasterfrom
use-castle-to-generate-delegate-proxy

Conversation

@zvirja
Copy link
Copy Markdown
Contributor

@zvirja zvirja commented Apr 8, 2019 •
edited
Loading

Recently @stakx did a favor to all of us and added possibility to generate delegate proxies to the Castle.Core library. Simplified our internal logic to use the Castle.Core generator for both delegates and classes.

Notice, I haven't removed the deprecated classes we already have, as it's not allowed by SemVer I'm trying to follow 😉 Instead, just deprecated all the types to remove them in future.

@dtchepak Please review the PR and let me know if you are fine. Don't merge it yet - I want to spend some time and benchmark the performance (after #536 is merged). Created it early, so we can discuss the approach 😉

UPD:

According to the benchmark results the activation performance is comparable:

BEFORE:

                           Method |  Job | Runtime |      Mean |     Error |    StdDev |    Median |  Gen 0 | Allocated |
--------------------------------- |----- |-------- |----------:|----------:|----------:|----------:|-------:|----------:|
        CreateInterfaceSubstitute |  Clr |     Clr |  4.970 us | 0.0993 us | 0.1816 us |  4.933 us | 0.9308 |   4.32 KB |
    CreateAbstractClassSubstitute |  Clr |     Clr |  4.545 us | 0.0859 us | 0.0882 us |  4.522 us | 0.8774 |   4.05 KB |
 CreateNonAbstractClassSubstitute |  Clr |     Clr |  4.496 us | 0.0817 us | 0.0764 us |  4.523 us | 0.8774 |   4.05 KB |
         CreateDelegateSubstitute |  Clr |     Clr | 11.112 us | 0.2137 us | 0.2099 us | 11.097 us | 0.9460 |   4.42 KB |
        CreateInterfaceSubstitute | Core |    Core |  4.864 us | 0.0965 us | 0.0991 us |  4.882 us | 1.0300 |   4.77 KB |
    CreateAbstractClassSubstitute | Core |    Core |  4.777 us | 0.0820 us | 0.0640 us |  4.776 us | 0.9766 |   4.51 KB |
 CreateNonAbstractClassSubstitute | Core |    Core |  4.926 us | 0.1127 us | 0.3324 us |  5.112 us | 0.9766 |   4.51 KB |
         CreateDelegateSubstitute | Core |    Core |  8.343 us | 0.1622 us | 0.2620 us |  8.296 us | 1.0529 |   4.87 KB |

AFTER:


                           Method |  Job | Runtime |      Mean |     Error |    StdDev |    Median |  Gen 0 | Allocated |
--------------------------------- |----- |-------- |----------:|----------:|----------:|----------:|-------:|----------:|
        CreateInterfaceSubstitute |  Clr |     Clr |  5.003 us | 0.0957 us | 0.0940 us |  4.975 us | 0.9384 |   4.33 KB |
    CreateAbstractClassSubstitute |  Clr |     Clr |  4.587 us | 0.0900 us | 0.1170 us |  4.579 us | 0.8774 |   4.06 KB |
 CreateNonAbstractClassSubstitute |  Clr |     Clr |  4.604 us | 0.0798 us | 0.0707 us |  4.590 us | 0.8774 |   4.06 KB |
         CreateDelegateSubstitute |  Clr |     Clr | 11.512 us | 0.1608 us | 0.1342 us | 11.488 us | 0.9918 |   4.58 KB |
        CreateInterfaceSubstitute | Core |    Core |  4.914 us | 0.0859 us | 0.0761 us |  4.907 us | 1.0376 |    4.8 KB |
    CreateAbstractClassSubstitute | Core |    Core |  4.426 us | 0.0856 us | 0.0801 us |  4.423 us | 0.9842 |   4.54 KB |
 CreateNonAbstractClassSubstitute | Core |    Core |  4.360 us | 0.0654 us | 0.0579 us |  4.358 us | 0.9842 |   4.54 KB |
         CreateDelegateSubstitute | Core |    Core |  8.090 us | 0.1618 us | 0.4591 us |  7.918 us | 1.0834 |   5.04 KB |

@zvirja zvirja requested a review from dtchepak April 8, 2019 21:18
dtchepak
dtchepak approved these changes Apr 13, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@dtchepak dtchepak left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@zvirja zvirja force-pushed the use-castle-to-generate-delegate-proxy branch from 3ce3a59 to a1cf685 Compare April 13, 2019 21:04
@zvirja zvirja force-pushed the use-castle-to-generate-delegate-proxy branch from a1cf685 to 1e665fd Compare April 13, 2019 21:06
@zvirja
Copy link
Copy Markdown
Contributor Author

zvirja commented Apr 13, 2019

@dtchepak Added benchmark results - everything is fine. Please take a look at the latest changes and fi you are fine - feel free to merge the code.

Thanks!

@zvirja zvirja changed the title [WIP] Use Castle to generate delegate proxies Use Castle to generate delegate proxies Apr 13, 2019
@dtchepak dtchepak merged commit 9a2e885 into master Apr 13, 2019
@dtchepak dtchepak deleted the use-castle-to-generate-delegate-proxy branch April 13, 2019 22:10
@kislovs kislovs mentioned this pull request Oct 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dtchepak dtchepak dtchepak approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@zvirja @dtchepak