Removed Method Group conversions, more additions to equality.#1030
Removed Method Group conversions, more additions to equality.#1030Mrnikbobjeff wants to merge 2 commits intoxamarin:mainfrom
Conversation
…conversions, but also some smaller things
|
When it comes to this will have @terrajobst validate. |
@jaredpar is more appropriate to validate this change |
terrajobst
left a comment
There was a problem hiding this comment.
Seems reasonable if true; @jaredpar would not know for sure.
|
Just for further reference, the current c# spec says the following about method group conversions in section 6.6 :"A new instance of the delegate type D is allocated. If there is not enough memory available to allocate the new instance, a System.OutOfMemoryException is thrown and no further steps are executed." |
|
Interesting... I always thought these anonymous delegates would be worse. Seems C# tries to catch me out even more ;) |
|
@jaredpar Did you have a change to look over the changes yet? |
|
@Mrnikbobjeff the method group conversions to delegates look good |
|
dotnet/roslyn#5835 I think it is related to this issue, though this is the roslyn compiler, I don't know if the same issue is in the mono compiler. |
|
That was the issue I referenced in the ticket. No matter what mono does, as long as it abides by the standard it will be exactly the same. Feel free to try yourself with the linked benchmark, you just have to add a ClrJob for mono |
Description of Change
Removed most method group conversions to be able to cache the delegate as this is currently forbidden by the standard for method group conversions. Here is the enum case benchmark as linked in the first issue, here we see the effect of method group conversions.
[Enhancement] Some struct equality comparisons still allocate. #1029, [Enhancement] Redundant allocation in TextToSpeech.Android #965
API Changes
None
Behavioral Changes
They should simply see less allocations throughout the app.
PR Checklist