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

Remove STA thread affinitisation (servicing) #5953

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Oct 12, 2021

Application bootstrap was designed and built before the file-scope namespaces became a thing, and supported
a) the original namespace declarations, and
b) the new top level statements declarations.

In the case of top level statements there is no Main() method on to which a developer can apply STAThreadAttribute, and to accommodate this use case we added explicit STA thread affinitisation.

The same code gets emitted for file-scope namespace declarations, which is undesirable, as it has overhead and perf implications.

Whilst the top level statements are supported in Windows Forms, those don't represent the main use case, and any developer who wishes to use top level statements in Program.cs can write STA thread affinitisation statements by hand.

  • Before
    image
  • After
    image

Customer Impact

  • Improved app start up

Regression?

  • No, Application.Initialize() is a new feature in .NET 6.0

Risk

  • Minimal
Microsoft Reviewers: Open in CodeFlow

Application bootstrap was designed and built before the file-scope namespaces
became a thing, and supported
a) the original namespace declarations, and
b) the new top level statements declarations.

In the case of top level statements there is no `Main()` method on to
which a developer can apply `STAThreadAttribute`, and to accommodate
this use case we added explicit STA thread affinitisation.

The same code gets emitted for file-scope namespace declarations, which
is undesirable, as it has overhead and perf implications.

Whilst the top level statements are supported in Windows Forms, those
don't represent the main use case, and any developer who wishes to use
top level statements in Program.cs can write STA thread affinitisation
statements by hand.
@RussKie RussKie added tenet-performance Improve performance, flag performance regressions across core releases Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Oct 12, 2021
@RussKie RussKie added this to the 6.0 rtm milestone Oct 12, 2021
@RussKie RussKie requested a review from a team as a code owner October 12, 2021 07:33
@ghost ghost assigned RussKie Oct 12, 2021
@RussKie RussKie requested a review from Pilchie October 12, 2021 07:33
@dreddy-work

This comment has been minimized.

@dreddy-work
Copy link
Member

dreddy-work commented Oct 12, 2021

Any issue associated with this for the background?

@dreddy-work

This comment has been minimized.

@leecow leecow added Servicing-approved .NET Shiproom approved the PR for merge and removed Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Oct 12, 2021
@RussKie
Copy link
Member Author

RussKie commented Oct 12, 2021

Any issue associated with this for the background?

I discovered this myself during exploratory testing of the the new feature.
For more background details these discussions may be useful:

Please let me know if you have any questions.

@dreddy-work dreddy-work added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Oct 13, 2021
@RussKie RussKie merged commit b7b6ae8 into dotnet:release/6.0 Oct 13, 2021
@RussKie RussKie deleted the revert_stathread_in_appinit6 branch October 13, 2021 22:59
@RussKie
Copy link
Member Author

RussKie commented Oct 13, 2021

@Olina-Zhang please add this into a verification runsheet. Thank you.

@RussKie RussKie removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Oct 13, 2021
@Olina-Zhang
Copy link
Member

@Olina-Zhang please add this into a verification runsheet. Thank you.

Will add it to regular checking.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Servicing-approved .NET Shiproom approved the PR for merge tenet-performance Improve performance, flag performance regressions across core releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants