Skip to content

Conversation

@maraf
Copy link
Member

@maraf maraf commented Dec 13, 2021

Problem description

  • When each new timer‡ is created it would start self-sustaining setTimeout loop.
  • The loop instance would only stop when ALL timers are done.
  • Many scheduled loops could be running in parallel to each other.
  • I added calls to mono_set_timeout_exec in Net6.0 to prevent timer throttling. That made the underlying problem worse and that's how it was detected.

‡ TimerQueue is group of timers, so only shorter than current in the same TimerQueue does that.

Fix

  • Port fix to the TimerQueue from [wasm] Reduce number of calls to setTimer #62433.
    • changes in TimerQueue.Browser.Mono.cs
      • we will always have only one, the shortest due setTimeout pending for all TimerQueue instances.
      • we remember it's due time in global s_shortestDueTimeMs
      • we will not schedule new setTimeout if current pending is earlier than requested one . Even from different TimerQueue instance.
      • if there is new timer which need to be scheduled earlier than the existing one, we would clear the previous one and register new one. Replace, rather than add the loop.
    • changes in src/mono/wasm/runtime/library_mono.js
      • added lastSchedule which is the previous registration of pending timer. We call clearTimeout on it, any time new timer is registered.

Testing

  • Test automated via simple.js but not run on CI.
    • Xharness version on this branch triggers timer queue in a way that makes the test inside Xharness useless.
  • the test would monkey patch globalThis.setTimeout to be able to observe number of setTimeout calls and number of callback executions.
  • it covers few basic test scenarios with expectations on number of calls mono does

Repro on blazor

number_of_setTimeout_calls

Profiling the app for 10 seconds every minute results in the below number of calls to "Install Timer".
00:30:00->00:40:00: 28 calls.
01:30:00->01:40:00: 27 calls.
02:30:00->02:40:00: 32 calls.
03:30:00->03:40:00: 25 calls.

Test with prevent_timer_throttling

prevent_timer_throttling

Number of scheduled timers is consistent in time (every 1sec).

TODO

  • run the repro
  • Test with prevent_timer_throttling

Fixes #62227

Test is only manual. Xharness version on this branch
triggers timer queue in a way that makes the test useless.

Co-authored-by: Pavel Savara <pavelsavara@microsoft.com>
@maraf maraf added the arch-wasm WebAssembly architecture label Dec 13, 2021
@maraf maraf requested a review from pavelsavara December 13, 2021 09:18
@maraf maraf requested a review from marek-safar as a code owner December 13, 2021 09:18
@ghost
Copy link

ghost commented Dec 13, 2021

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

Issue Details

Port changes to the TimerQueue from #62433.

Test is only manual. Xharness version on this branch triggers timer queue in a way that makes the test useless.

Author: maraf
Assignees: -
Labels:

arch-wasm

Milestone: -

@pavelsavara
Copy link
Member

The main simple of the simple test was used instead of the Xunit main Log.

We will have to make it normal static method and bind it as such in the test script.

@pavelsavara pavelsavara requested a review from kg December 13, 2021 19:04
@pavelsavara
Copy link
Member

received approval by email.

@marek-safar marek-safar added the Servicing-approved Approved for servicing release label Dec 15, 2021
@safern safern merged commit 7d82ac7 into dotnet:release/6.0 Jan 3, 2022
@maraf maraf deleted the WasmFixTimersOn60 branch January 3, 2022 07:30
@ghost ghost locked as resolved and limited conversation to collaborators Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants