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

Unnecessary GuidGenerator calls for custom delegates in authored types #1499

Open
Sergio0694 opened this issue Feb 4, 2024 · 2 comments
Open
Labels
performance Related to performance work trimming

Comments

@Sergio0694
Copy link
Member

Sergio0694 commented Feb 4, 2024

Describe the bug

We're emitting some GuidGenerator calls when using custom delegates in authored components:

image

Pretty sure we could just literally hardcoded that same GUID we added on the ABI type for the delegate instead?

To Reproduce

public sealed unsafe partial class HelloWorldEffect
{
    public event MyEventHandler Foo;
}

public delegate void MyEventHandler(object sender, string args);

Expected behavior

We should probably emit that IID in a static readonly field, and then reference it wherever needed.

Version Info

2.0.5-prerelease.240203.7

@Sergio0694 Sergio0694 added performance Related to performance work trimming labels Feb 4, 2024
@manodasanW
Copy link
Member

This is probably getting optimized by IIDOptimizer today, but I agree we can move this to a readonly static field.

@Sergio0694
Copy link
Member Author

If that's the case I think we should probably also fix #1478? Because otherwise IIDOptimizer will break in libraries, and if you just opt-out, then that path will not be optimized by it. I mean I agree here we can just update the code writer, but this just makes me think that in general, the IIDOptimizer should also run on authored libraries then? 🤔

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

No branches or pull requests

2 participants