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

pal init: Init membarrier() before first thread to improve start time #106724

Merged

Conversation

harisokanovic
Copy link
Contributor

@harisokanovic harisokanovic commented Aug 20, 2024

pal init: InitializeFlushProcessWriteBuffers() before first thread to improve start time

InitializeFlushProcessWriteBuffers() initializes expedited membarrier()
syscall on Linux, which is much slower when called in a multi-thread
process. Move this init earlier to improve dotnet process start time.
A detailed explanation can be found in issue 106722.

Fixes #106722

@harisokanovic harisokanovic force-pushed the dev/harisokn/improve-membarrier-init branch from 10107a0 to 27896a2 Compare August 21, 2024 18:54
… improve start time

InitializeFlushProcessWriteBuffers() initializes expedited membarrier()
syscall on Linux, which is much slower when called in a multi-thread
process. Move this init earlier to improve dotnet process start time.
A detailed explanation can be found in issue 106722.

Fixes dotnet#106722
@harisokanovic harisokanovic force-pushed the dev/harisokn/improve-membarrier-init branch from 27896a2 to bc7dbdd Compare August 21, 2024 19:12
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you

Could you please accept the Contributor License Agreement?

@harisokanovic
Copy link
Contributor Author

@dotnet-policy-service agree company="Amazon"

@jkotas

This comment was marked as resolved.

@harisokanovic

This comment was marked as resolved.

@harisokanovic
Copy link
Contributor Author

The test failures appear to be intermittent per #100558 and #2496. I don't have permission to rerun those jobs.

@jkotas jkotas merged commit 27ee590 into dotnet:main Aug 22, 2024
87 of 90 checks passed
@harisokanovic
Copy link
Contributor Author

@jkotas, is it possible to backport this change to dotnet8?
CC @Beau-Gosse-dev

@jkotas
Copy link
Member

jkotas commented Aug 22, 2024

/backport to release/9.0

Copy link
Contributor

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10514758065

@jkotas
Copy link
Member

jkotas commented Aug 22, 2024

is it possible to backport this change to dotnet8?

I can backport it to .NET 9 RC for now.

This change does not meet our default servicing bar. It is a perf fix that is not fixing a regression, the problem existed for number of years since membarrier was introduced. Given that the fix is very simple and it provides measurable improvement for every app, I can ask for an exception for .NET 8 backport after the fix bakes for several months in main and .NET 9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member tenet-performance Performance related issue
Projects
None yet
3 participants