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

[release/6.0-rc1] [wasm] spread WS based timers over next 6 minutes to prevent heavy throttling #58160

Merged
merged 8 commits into from
Aug 26, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 26, 2021

Backport of #57745 to release/6.0-rc1

Fixes #51041

/cc @pavelsavara

Customer Impact

As discussed by community in #51041, the way how Chromium throttles inactive/hidden pages is causing problems in reliability of SignalR or other code which relies on dotnet Timers.
https://developer.chrome.com/blog/timer-throttling-in-chrome-88/#intensive-throttling

If the page received WS message in last 6 minutes, the fix prevents browser from heavy throttling (60sec). Light throttling to timer tick (1sec) could not be prevented.

This PR improves dotnet timers behavior in those scenarios, but it does not eliminate all throttling.

Testing

It needs to be tested with browser with UI, headless browsers do not throttle the timers.
Automated test written as part of the PR, executed on developer machine that proves it works.
Tested with Chrome, Edge.

The code is executed on all unit tests of WASM implementation of ClientWebSocket in our CI pipeline. That should prove that the PR doesn't break any existing functionality.
Also tested with with Firefox locally, which is not addressed by this PR.

Risk

It introduces small method with new logic prevent_timer_throttling which in turn calls existing methods in the mono VM, to execute pending timers and threadpool jobs.

@ghost
Copy link

ghost commented Aug 26, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #57745 to release/6.0-rc1

/cc @pavelsavara

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net

Milestone: -

@pavelsavara pavelsavara requested review from lewing and kg August 26, 2021 07:48
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture Servicing-consider Issue for next servicing release review labels Aug 26, 2021
@ghost
Copy link

ghost commented Aug 26, 2021

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

Issue Details

Backport of #57745 to release/6.0-rc1

/cc @pavelsavara

Customer Impact

As discussed by community in #51041, the way how Chromium throttles inactive/hidden pages is causing problems in reliability of SignalR or other code which relies on dotnet Timers.

If the page received WS message in last 6 minutes, the fix prevents browser from heavy throttling (60sec). Light throttling to timer tick (1sec) could not be prevented.

This PR improves dotnet timers behavior in those scenarios, but it does not bring it on par with desktop targets.

Testing

Automated test on developer machine that proves it works.
Tested with Chrome, Edge.

The code is executed on all unit tests of WASM implementation of ClientWebSocket in our CI pipeline. That should prove that the PR doesn't break any existing functionality.
Also with Firefox which is not addressed by this PR.

Risk

It introduces small method with new logic prevent_timer_throttling which in turn calls existing methods in the mono VM, to execute pending timers and threadpool jobs.

Author: github-actions[bot]
Assignees: -
Labels:

Servicing-consider, arch-wasm, area-System.Net

Milestone: -

@pavelsavara
Copy link
Member

Unrelated failure

D:\workspace\_work\1\s\src\coreclr\jit\valuenum.cpp(10788): fatal error C1090: PDB API call failed, error code '23': (0x000006BA)
[640/2334] Building CXX object jit\CMakeFiles\clrjit_unix_arm64_arm64.dir\simd.cpp.obj
ninja: build stopped: subcommand failed.

@karelz karelz added this to the 6.0.0 milestone Aug 26, 2021
@lewing
Copy link
Member

lewing commented Aug 26, 2021

runtime (CoreCLR Product Build windows arm64 release) failure is #48070

@lewing lewing added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 26, 2021
@lewing
Copy link
Member

lewing commented Aug 26, 2021

approved in email

@Anipik Anipik merged commit c507117 into release/6.0-rc1 Aug 26, 2021
@akoeplinger akoeplinger deleted the backport/pr-57745-to-release/6.0-rc1 branch August 28, 2021 22:03
@ghost ghost locked as resolved and limited conversation to collaborators Sep 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants