Skip to content

[browser][MT] use portable timer - #94180

Merged
pavelsavara merged 3 commits into
dotnet:mainfrom
pavelsavara:browser_mt_portable_timer
Oct 31, 2023
Merged

[browser][MT] use portable timer#94180
pavelsavara merged 3 commits into
dotnet:mainfrom
pavelsavara:browser_mt_portable_timer

Conversation

@pavelsavara

@pavelsavarapavelsavara commented Oct 30, 2023•
edited
Loading

Copy link
Copy Markdown
Member

TimerQueue ST implementation relies on setTimeout (which deadlocks in MT)

  • we are moving TimerQueue from MT+ST to ST only.
  • After this change, MT will use portable timer (same as any Unix), which will create dedicated thread.

@pavelsavarapavelsavara added arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser variant of arch-wasm labels Oct 30, 2023
@pavelsavarapavelsavara added this to the 9.0.0 milestone Oct 30, 2023
@pavelsavarapavelsavara self-assigned this Oct 30, 2023
@ghost

ghost commented Oct 30, 2023

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author:pavelsavara
Assignees:pavelsavara
Labels:

arch-wasm, area-VM-threading-mono, os-browser

Milestone:9.0.0

@pavelsavara

pavelsavara commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Oct 30, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara

pavelsavara commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Oct 30, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@kg

kg commented Oct 31, 2023

Copy link
Copy Markdown
Contributor

This looks like timers will only ever run on the main thread, is that a problem? I don't recall whether timers work that way on other platforms

kg
kg approved these changes Oct 31, 2023
View reviewed changes
ilonatommy
ilonatommy reviewed Oct 31, 2023
View reviewed changes
Comment threadsrc/mono/System.Private.CoreLib/System.Private.CoreLib.csproj
@pavelsavara
pavelsavara merged commit 80b08ed into dotnet:mainOct 31, 2023
@pavelsavara
pavelsavara deleted the browser_mt_portable_timer branch October 31, 2023 11:32
@pavelsavara

pavelsavara commented Oct 31, 2023

Copy link
Copy Markdown
MemberAuthor

This looks like timers will only ever run on the main thread, is that a problem? I don't recall whether timers work that way on other platforms

We discussed this elsewhere, portable timer in MT will create dedicated thread. ST will stay using setTimeout

@pavelsavarapavelsavara mentioned this pull request Oct 31, 2023
Merged
This was referenced Nov 8, 2023
Closed
Closed
Closed
@ghostghost locked as resolved and limited conversation to collaborators Nov 30, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@ilonatommyilonatommyilonatommy approved these changes
@lambdageeklambdageekAwaiting requested review from lambdageek
@vargazvargazAwaiting requested review from vargaz
@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRT
@marek-safarmarek-safarAwaiting requested review from marek-safar
+1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@pavelsavarapavelsavara

Labels

arch-wasmWebAssembly architecturearea-VM-threading-monoos-browserBrowser variant of arch-wasm

Projects

None yet

Milestone

9.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@pavelsavara@kg@ilonatommy