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

Performance vs postsharp #97

Closed
hhblaze opened this issue Mar 13, 2017 · 17 comments
Closed

Performance vs postsharp #97

hhblaze opened this issue Mar 13, 2017 · 17 comments

Comments

@hhblaze
Copy link

hhblaze commented Mar 13, 2017

Hi, our team uses postsharp already about 8 years (free version) to wrap any method/property into try-catch and measure quantity of calls, execution time and not finished calls, inside of our modules.

Now Postsharp tends to be not exactly free, so I am investigation new approaches like Mr.Advice.
I have re-weaved one of the modules from Postsharp to Mr.Advice and execution speed of specific measured methods has dropped down to 4-5 times (from 100ms to 500ms). Investigation brought me to the understanding, that the Mr.Advice generated byte code is a problem.

Postsharp integrates new byte code directly into the method, when Mr.Advice makes a 2 call chain and also makes extra type casting. When it's necessary to create many objects (e.g. 1000 with 20 properties each) inside of a method (where every object is under control) Mr.Advice makes 1000 * 20 * 3 calls + type casting, when for the Postsharp it' is enough only 1000 * 20 calls and no type casting.

Please, check examples down in the text.

Question is it possible to make optimization there?

Example:

Postsharp weaved property
https://gist.github.com/hhblaze/01b7e4caf49c56f992c865d1b6829330

Mr.Advice call chain
https://gist.github.com/hhblaze/8671da67e362bcf775f2a7dbdef64976
https://gist.github.com/hhblaze/5c3f03a0d37dde2f2526794f3d239319
https://gist.github.com/hhblaze/8b81af75f8d225f231282bd622ba34f8

@picrap
Copy link
Member

picrap commented Mar 13, 2017

Hi Alexey,

anything is possible, especially when there is a challenge 😄
I never even tried to profile Mr. Advice code, so I think I'll start with that.
Also, if you have more specific ideas, I'm listening (well... Reading...).
The extra call can be disabled, however I wrote it with idea to reduce code size in mind. Profiling will tell if the idea is that bad.

Let's keep this issue open and use it to share ideas.

Pascal.

@hhblaze
Copy link
Author

hhblaze commented Mar 13, 2017

Well, I think you have made so much for this project that it gonna be a good idea to make the implementation perfect. For such library optimization is vital. For my dev. team, getting rid of the PostSharp as an AOP flagman and substitute it with something fast and open is a gift!
In the compile chain after Mr.Advice was used https://github.com/yck1509/ConfuserEx - they both work like a charm.

@hhblaze
Copy link
Author

hhblaze commented Mar 15, 2017

Some more optimization thoughts (may be it's already possible, but I don't know how to achieve that).

In my scenario the advice is applied on the complete assembly. I have only one IMethod advice, but it's applied to all methods, properties and .actors after weaving (it is correct). Mentioned method (with dropped down performance) has a loop inside that creates 1000 objects of one type. This type is a POCO class with 20 properties and a constructor. Properties are quite simple, something like "public bool MyProperty {get; set;}" and I am ready to declare them as safe.
Mr.Advice applies the advice on each of them. I would like to exclude those properties from weaving, but without un-decorating each of them. I know that there is an exclusion attribute in Mr.Advice, but is there a way to exclude from weaving class only properties leaving methods? So, such attribute must be on the level of a class.
And finally, because in my case objects are POCO and designed to be serialized and moved between different software tiers, not any of the tier has Mr.Advice installed, but such classes must be compiled without Mr.Advice. It (may be) should mean that there going to be a way to create decorating attributes outside of Mr.Advice and then supply them to Mr.Advice via configuration.

@picrap
Copy link
Member

picrap commented Mar 15, 2017

Dude! Please open other issues (I see two in your message: a question about excluding properties and a suggestion about referenceless advising).
I did some optimizations yesterday, the harder are still to come 😦

@hhblaze
Copy link
Author

hhblaze commented Mar 15, 2017

Done!

@picrap
Copy link
Member

picrap commented Mar 15, 2017

I just released a version 2.4.8 where calls have been optimized by a factor of 2.5.
The next big thing to test is to replace MethodBase.Invoke which takes most of the remaining delay.

@TheXenocide
Copy link

TheXenocide commented Mar 15, 2017

I believe IL supports invoking Method Handles in a way that doesn't translate to C# well, but is functionally equivalent to using a known function pointer instead of a managed delegate or other late bound invocation (as in Reflection). It has been a long time since I looked at the IL spec so I forget exactly how it's done, but it might be a key optimization for that

@picrap
Copy link
Member

picrap commented Mar 16, 2017

There is no call* IL instruction that allows to specify the target method as an argument. All of them use the method in an operand.
So currently, I'm thinking to pass a Delegate to invocation. This may require writing another method whose purpose is to transform arguments array to real arguments and them to the final (original) method.
So we would have in the call chain:

  1. The wrapper code in the original method signature.
  2. The Mr.Advice Invocation.Proceed call (direct or indirect)
  3. A new method taking one argument of type object[] (this one containing all original or modified arguments). I will refer this as the argument unpacker. And this one will be given as the delegate to Invocation.Proceed.
  4. The final execution point containing the original code.

Edit: some performance comparison here: http://www.abhisheksur.com/2010/11/reflection-slow-or-faster-demonstration.html

@picrap
Copy link
Member

picrap commented Mar 26, 2017

@hhblaze did you test the latest version? Some performance improvements were made.

@hhblaze
Copy link
Author

hhblaze commented Mar 27, 2017

Thank you, will test ASAP

@hhblaze
Copy link
Author

hhblaze commented Mar 27, 2017

Ok, the same function middle execution time:
Postsharp 193 ms
MrAdvice 226 ms

Now, may be, you can in popular way to explain me, how should I exclude [CompilerGenerated] on the level of Aspect and MrAdvice will definitely beat postsharp

@picrap
Copy link
Member

picrap commented Mar 27, 2017

Great! We will burn them once invocation with delegates is implemented (instead of MethodBase.Invoke)!
Regarding [CompilerGenerated], this is not implemented yet 😞

@hhblaze
Copy link
Author

hhblaze commented Mar 27, 2017

Very well, ....waiting for improvements and implementations...

@picrap
Copy link
Member

picrap commented Apr 16, 2017

I just released a 2.5 version of Mr. Advice which includes the final optimization I could think of: using delegates instead of reflection to invoke advised method.
We are now roughly 3.2 times faster than the original implementation.
Let me know with your own measures, afterwards we'll close this issue.

@hhblaze
Copy link
Author

hhblaze commented Apr 16, 2017

Very well, ASAP

@hhblaze
Copy link
Author

hhblaze commented Apr 18, 2017

Ok. In a set of tests Postsharp needs 107 ms MrAdvice needs - 80 ms.
It's a victory!
I close this ticket and still waiting for [CompilerGenerated] solution

Thank you very much!

@hhblaze hhblaze closed this as completed Apr 18, 2017
@picrap
Copy link
Member

picrap commented Apr 18, 2017

I love the smell of PostSharp humiliation ✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants