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

[browser][MT] use portable timer #94180

Merged
merged 3 commits into from
Oct 31, 2023
Merged

[browser][MT] use portable timer #94180

merged 3 commits into from
Oct 31, 2023

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Oct 30, 2023 •
edited
Loading

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.

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

ghost commented Oct 30, 2023

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
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kg
Copy link
Contributor

kg commented Oct 31, 2023

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

@pavelsavara pavelsavara merged commit 80b08ed into dotnet:main Oct 31, 2023
119 checks passed
@pavelsavara pavelsavara deleted the browser_mt_portable_timer branch October 31, 2023 11:32
@pavelsavara
Copy link
Member Author

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

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

@kg kg kg approved these changes

@ilonatommy ilonatommy ilonatommy approved these changes

@lambdageek lambdageek Awaiting requested review from lambdageek lambdageek is a code owner

@vargaz vargaz Awaiting requested review from vargaz

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

Assignees

@pavelsavara pavelsavara

Labels
arch-wasm WebAssembly architecture area-VM-threading-mono os-browser Browser 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