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

Consider emitting methods with SkipLocalsInitAttribute #595

Open
AArnott opened this issue Jun 24, 2022 · 8 comments
Open

Consider emitting methods with SkipLocalsInitAttribute #595

AArnott opened this issue Jun 24, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@AArnott
Copy link
Member

AArnott commented Jun 24, 2022

The LibraryImportAttribute source generator emits code that applies SkipLocalsInitAttribute to each method. Perhaps we should too for perf reasons.

@AArnott AArnott added the enhancement New feature or request label Jun 24, 2022
@elachlan
Copy link
Contributor

There are a couple of scenarios to consider when outputting it.
https://www.meziantou.net/csharp-9-improve-performance-using-skiplocalsinit.htm

@elachlan
Copy link
Contributor

Do we only want to apply SkipLocalsInitAttribute to methods and not classes/structs?

@AArnott
Copy link
Member Author

AArnott commented Aug 19, 2022

We can completely control the methods we emit to ensure they are safe. Structs we emit are expected to be initialized by the users, which are accustomed to .NET behavior of clearing memory for a new struct, so I do not think we should apply this attribute on structs.
And I don't think we emit any classes (with fields, anyway).

@jnm2
Copy link
Contributor

jnm2 commented Aug 22, 2022

Is there any difference between placing [SkipLocalsInit] on a type and placing it only on each of the type's methods and accessors (and nested types)?

@elachlan
Copy link
Contributor

From the docs. I actually had no idea. It does apply to all methods. It doesn't say it applied to the fields on the module though.
https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.skiplocalsinitattribute?view=net-6.0

This attribute is unsafe, because it may reveal uninitialized memory to the application in certain instances (for example, reading from uninitialized stack-allocated memory). If applied to a method directly, the attribute applies to that method and all its nested functions, including lambdas and local functions. If applied to a type or module, it applies to all methods nested inside. This attribute is intentionally not permitted on assemblies. To apply the attribute to multiple type declarations, use it at the module level instead.

@jnm2
Copy link
Contributor

jnm2 commented Aug 23, 2022

How could it apply to fields? It only affects the locals block of method bodies so that they are not locals-init blocks.

@AArnott
Copy link
Member Author

AArnott commented Aug 23, 2022

I think you're right. I am not too familiar with the attribute yet so I probably added more confusion than clarity with my last comment.

@JeremyKuhne
Copy link
Member

One other option here is to use Unsafe.SkipInit() explicitly on structs that don't need initialization. That doesn't help for stackalloc of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants