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.

Sign up for GitHub

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

Jump to bottom

Move System.Threading.Event/Mutex/Semaphore support to managed code #44795

Closed
vargaz opened this issue Nov 17, 2020 · 4 comments · Fixed by #47333
Closed

Move System.Threading.Event/Mutex/Semaphore support to managed code #44795

vargaz opened this issue Nov 17, 2020 · 4 comments · Fixed by #47333
Assignees
@CoffeeFlux
Labels
area-VM-threading-mono size-reduction Issues impacting final app size primary for size sensitive workloads
Milestone
6.0.0

Comments

@vargaz
Copy link
Contributor

vargaz commented Nov 17, 2020

These synchronization classes have little to do with the runtime and they can be implemented mostly in managed code as done in corert.
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Threading untriaged New issue has not been triaged by the area owner labels Nov 17, 2020
@vargaz vargaz added area-CoreLib-mono and removed untriaged New issue has not been triaged by the area owner area-System.Threading labels Nov 17, 2020
@vargaz vargaz self-assigned this Nov 17, 2020
@marek-safar marek-safar added this to the 6.0.0 milestone Nov 19, 2020
@vargaz vargaz removed their assignment Dec 8, 2020
@jkotas jkotas mentioned this issue Dec 18, 2020
Open
@CoffeeFlux CoffeeFlux self-assigned this Jan 21, 2021
@CoffeeFlux
Copy link
Contributor

CoffeeFlux commented Jan 21, 2021 •
edited
Loading

Related: mono/mono#8237
WaitSubsystem makes use of EagerStaticClassConstruction.

@CoffeeFlux CoffeeFlux mentioned this issue Jan 21, 2021
Open
23 tasks
@marek-safar
Copy link
Contributor

Could they be rewritten into something else or use C# module initializers instead?

@jkotas
Copy link
Member

jkotas commented Jan 21, 2021 •
edited
Loading

WaitSubsystem makes use of EagerStaticClassConstruction.

It is because of the synchronization of static constructors depends on WaitSubsystem in CoreRT. I do not expect that you will have nor want to introduce this dependency in Mono.

It should be fine to just put use of this attribute under #if CORERT ifdef.

@CoffeeFlux CoffeeFlux mentioned this issue Jan 21, 2021
Closed
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 21, 2021
@monojenkins monojenkins mentioned this issue Jan 21, 2021
Closed
monojenkins pushed a commit to monojenkins/mono that referenced this issue Jan 21, 2021
@CoffeeFlux
Port some CoreRT Threading classes to Mono
6a61c1e 
Draft until I see how this behaves on CI.

Fixes dotnet/runtime#44795

A few notes:

I've attempted to nullable annotate the relevant files but would appreciate someone else checking over my work, since the design does not really lend itself to that.

I can split the time-related parts of the System.Native addition off into a separate header if desired, but since they wouldn't be used elsewhere I just put the relevant includes in the C file.

This can fairly easily be moved to shared if desired, but I've put it in Mono only for now. Given there's a chance we might want to share it eventually after testing in Mono, I've left a CORERT define.

The `TARGET_UNIX` define usage isn't ideal, but given we care about the layout for that particular class it seemed like the easiest option. If it's a problem please let me know.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 22, 2021
@CoffeeFlux CoffeeFlux mentioned this issue Jan 22, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 22, 2021
@monojenkins monojenkins mentioned this issue Jan 22, 2021
Closed
monojenkins pushed a commit to monojenkins/mono that referenced this issue Jan 22, 2021
@CoffeeFlux
Port some CoreRT Threading classes to Mono
ac1021c 
Depends on dotnet/runtime#47325 and dotnet/runtime#47327, draft until they're in and I rebase this.

Fixes dotnet/runtime#44795

Best reviewed commit by commit—the commits starting with "Remote appropriate icalls from Mono" are new. In some cases, I've left comments in the commit description. I expect the most interesting commits to be the last few, in particular the annotations.
@CoffeeFlux CoffeeFlux added the size-reduction Issues impacting final app size primary for size sensitive workloads label Feb 16, 2021
@ghost
Copy link

ghost commented Feb 16, 2021

Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar, @tannergooding, @CoffeeFlux
See info in area-owners.md if you want to be subscribed.

Issue Details

These synchronization classes have little to do with the runtime and they can be implemented mostly in managed code as done in corert.

Author: vargaz
Assignees: CoffeeFlux
Labels:

area-Threading-mono, in pr, size-reduction
Milestone: 6.0.0

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@CoffeeFlux CoffeeFlux

Labels
area-VM-threading-mono size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Milestone
6.0.0
5 participants
@vargaz @marek-safar @CoffeeFlux @jkotas @Dotnet-GitSync-Bot