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

[mono] Perform the class init at runtime when static fields of BEFORE_FIELD_INIT type are accessed #86787

Closed
wants to merge 14 commits into from

Conversation

LeVladIonescu
Copy link
Contributor

@LeVladIonescu LeVladIonescu commented May 26, 2023

Changed class init checks executed at runtime when accessing static fields of beforefieldinit types, instead of running the cctors during method compilation.

Related to #77513.

Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-5.local>
@BrzVlad
Copy link
Member

BrzVlad commented May 26, 2023

The commit description is pretty confusing and is referencing code instead of being more of an explanation. Something like: Emit class init checks executed at runtime when accessing static fields of beforefieldinit types, instead of running the cctors during method compilation.

@LeVladIonescu LeVladIonescu changed the title [mono] Ignore beforefieldinit check [mono] Perform the class init at runtime when static fields of BEFORE_FIELD_INIT type are accessed May 26, 2023
@EgorBo
Copy link
Member

EgorBo commented May 26, 2023

NOTE: we keep this path in Boolean.cs because Mono could trigger ICU initialization because of this beforefieldinit behavior so in theory we can drop that path if you fix this. Although, it might regress perf slightly

@LeVladIonescu
Copy link
Contributor Author

LeVladIonescu commented May 26, 2023

@BrzVlad thanks for pointing it out, will keep it in mind.
Updated it.

@vargaz
Copy link
Contributor

vargaz commented May 31, 2023

This could use a testcase.

…tic fields

Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-5.local>
@LeVladIonescu
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Vlad - Alexandru Ionescu and others added 2 commits June 1, 2023 16:54
… the class

Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-5.local>
@LeVladIonescu
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the test project really need to run separately from other tests?

Signed-off-by: Vlad - Alexandru Ionescu <vlad-alexandruionescu@Vlads-MacBook-Pro-5.local>
@LeVladIonescu
Copy link
Contributor Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stephentoub
Copy link
Member

@lambdageek, what should be done with this PR?

@lambdageek
Copy link
Member

I feel like this is kind of risky to take for .NET 9 at this point. Static cctors order changes are hard to diagnose downstream. We could take it up early in .NET 10

@lambdageek lambdageek closed this Jul 22, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants