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

Do not consider final method overridable #223

Merged
merged 2 commits into from
May 2, 2024

Conversation

billybraga
Copy link
Contributor

@billybraga billybraga commented Nov 3, 2023

As noted here, "To determine if a method is overridable, it is not sufficient to check that IsVirtual is true. For a method to be overridable, IsVirtual must be true and IsFinal must be false."

I may be wrong, but I think this PR may increase performance and keep functionnality intact.

(By the way, I submitted another PR by mistake earlier, trying to update my fork, and it seems to have linked a bunch of stuff to it, sorry)

I did a small benchmark:

BenchmarkRunner.Run<Benchmark>();
return;

public interface I
{
    string? P { get; set; }
}

public class C : I
{
    public string? P { get; set; }
}

public class Benchmark
{
    static readonly Type Type = typeof(C);
    
    static readonly MethodInfo MethodInfo = Type.GetProperty(nameof(C.P))?.GetMethod
        ?? throw new ArgumentException("Did not find method");

    [Benchmark]
    public void DecompileNew()
    {
        MethodBodyDecompiler.Decompile(MethodInfo, Type);
    }
    
    [Benchmark]
    public void DecompileOld()
    {
        MethodBodyDecompiler.DecompileOld(MethodInfo, Type);
    }
}
| Method       | Mean         | Error      | StdDev      |
|------------- |-------------:|-----------:|------------:|
| DecompileNew |     3.030 us |  0.0871 us |   0.2554 us |
| DecompileOld | 2,292.221 us | 60.4400 us | 175.3475 us |

@billybraga
Copy link
Contributor Author

@hazzik Could this be merged?

@hazzik hazzik enabled auto-merge (squash) May 2, 2024 10:49
@hazzik hazzik merged commit ca3acc7 into hazzik:main May 2, 2024
1 check passed
@billybraga billybraga deleted the consider-final-methods-as-concrete branch May 2, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants