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

[wasm] spread WS based timers over next 6 minutes to prevent heavy throttling #57745

Merged
merged 8 commits into from
Aug 26, 2021
Merged

[wasm] spread WS based timers over next 6 minutes to prevent heavy throttling #57745

merged 8 commits into from
Aug 26, 2021

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Aug 19, 2021 •
edited
Loading

Problem

Chromium browsers throttle setTimeout frequency on inactive pages to 1 sec almost immediately.
5 minutes later they throttle heavily to wakeup each 60 seconds.
That breaks dotnet Timer expectations and also threadpool.

The heavy throttling could be lifted back to 1sec if there was WebSocket activity.
https://developer.chrome.com/blog/timer-throttling-in-chrome-88/#intensive-throttling

Implementation

The idea is to cover the future 6 minutes with "wakeup" timer, each 1 second apart from each other.
1 second, because is the light throttling rate, so it would not help to try to do it more often.
It should be triggered from WS event, so that it's only 1 deep from it.
Every subsequent call would schedule only for uncovered future.

There is [OuterLoop] unit test for this.

Fixes #51041

@pavelsavara pavelsavara added the arch-wasm WebAssembly architecture label Aug 19, 2021
@pavelsavara pavelsavara requested review from lewing and kg August 19, 2021 17:27
@ghost
Copy link

ghost commented Aug 19, 2021

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

Issue Details

Fixes #51041

Author: pavelsavara
Assignees: -
Labels:

arch-wasm
Milestone: -

@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 19, 2021 •
edited
Loading

Right now, I'm not clear if 6 minutes is right time span.
Also I plan to implement OuterLoop unit test for it yet.

@pavelsavara
Copy link
Member Author

This is necessary infrastructure change for the automated test. It's still quite fragile.
dotnet/xharness#695

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@runfoapp runfoapp bot mentioned this pull request Aug 23, 2021
Open
@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 23, 2021 •
edited
Loading

New xharness binaries are in.

@pavelsavara pavelsavara marked this pull request as ready for review August 23, 2021 15:49
@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

Outerloop issue is Unable to open X display. for the new System.Net.WebSockets.Client.Wasm.Tests, which makes sense.
But throttling is not testable without real visible browser UI.

@kg
Copy link
Member

kg commented Aug 25, 2021

IIRC there are ways to run chromium headless with a 'fake' framebuffer, window etc. Not sure which one we'd use here, or whether it would actually trigger this. Another option would be a headless X instance paired to VNC, I know people do that for tests sometimes.

@pavelsavara
Copy link
Member Author

I just excluded the test from CI, making it manual test.

kg
kg approved these changes Aug 25, 2021
View reviewed changes
Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

Everything looks fine to me

src/libraries/System.Net.WebSockets.Client/System.Net.WebSockets.Client.sln Outdated Show resolved Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/wasm/BrowserTimerThrottlingTest.cs Outdated Show resolved Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/wasm/BrowserTimerThrottlingTest.cs Outdated Show resolved Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/wasm/BrowserTimerThrottlingTest.cs Outdated Show resolved Hide resolved
src/libraries/System.Net.WebSockets.Client/tests/wasm/BrowserTimerThrottlingTest.cs Show resolved Hide resolved
src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved
src/mono/wasm/runtime/binding_support.js Outdated Show resolved Hide resolved
src/mono/wasm/runtime/library_mono.js Outdated Show resolved Hide resolved
@pavelsavara pavelsavara merged commit 5eaa911 into dotnet:main Aug 26, 2021
@pavelsavara
Copy link
Member Author

/backport to release/6.0-rc1

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/runtime/actions/runs/1169555771

@github-actions github-actions bot mentioned this pull request Aug 26, 2021
Merged
@karelz karelz added this to the 7.0.0 milestone Aug 26, 2021
This was referenced Aug 26, 2021
Closed
Closed
Closed
@karelz karelz mentioned this pull request Sep 9, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 25, 2021
@pavelsavara pavelsavara deleted the wasm_prevent_timer_throttling branch January 4, 2022 13:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kg kg kg approved these changes

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

@marek-safar marek-safar Awaiting requested review from marek-safar

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

[WASM] Chrome timer throttling affects runtime
3 participants
@pavelsavara @kg @karelz