Skip to content

runtime: modified timer results in extreme cpu load [1.17 backport] #53846

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
Closed
gopherbot opened this issue Jul 13, 2022 · 3 comments
Closed

runtime: modified timer results in extreme cpu load [1.17 backport] #53846

gopherbot opened this issue Jul 13, 2022 · 3 comments
Assignees
@prattmic
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone
Go1.17.13

Comments

@gopherbot
Copy link
Contributor

@prattmic requested issue #51654 to be considered for backport to the next 1.17 minor release.

@gopherbot Please backport to 1.17 and 1.18. This is a serious issue (busy loop in scheduler for up to 2 minutes) with no reasonable workaround.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jul 13, 2022
@gopherbot gopherbot mentioned this issue Jul 13, 2022
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@gopherbot gopherbot added this to the Go1.17.13 milestone Jul 13, 2022
@mknyszek mknyszek moved this to In Progress in Go Compiler / Runtime Jul 13, 2022
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/417476 mentions this issue: [release-branch.go1.17] runtime: clear timerModifiedEarliest when last timer is deleted

@toothrot toothrot added the CherryPickApproved Used during the release process for point releases label Jul 20, 2022
@toothrot
Copy link
Contributor

Approved. This is a serious issue with no workaround.

@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jul 20, 2022
@gopherbot
Copy link
Contributor Author

Closed by merging 66c60f0 to release-branch.go1.17.

Repository owner moved this from In Progress to Done in Go Compiler / Runtime Jul 25, 2022
gopherbot pushed a commit that referenced this issue Jul 25, 2022
@prattmic @cherrymui
[release-branch.go1.17] runtime: clear timerModifiedEarliest when las…
66c60f0 
…t timer is deleted

timerModifiedEarliest contains the lowest possible expiration for a
modified earlier timer, which may be earlier than timer0When because we
haven't yet updated the heap. Note "may", as the modified earlier timer
that set timerModifiedEarliest may have since been modified later or
deleted.

We can clear timerModifiedEarliest when the last timer is deleted
because by definition there must not be any modified earlier timers.

Why does this matter? checkTimersNoP claims that there is work to do if
timerModifiedEarliest has passed, causing findRunnable to loop back
around to checkTimers. But the code to clean up timerModifiedEarliest in
checkTimers (i.e., the call to adjusttimers) is conditional behind a
check that len(pp.timers) > 0.

Without clearing timerModifiedEarliest, a spinning M that would
otherwise go to sleep will busy loop in findRunnable until some other
work is available.

Note that changing the condition on the call to adjusttimers would also
be a valid fix. I took this approach because it feels a bit cleaner to
clean up timerModifiedEarliest as soon as it is known to be irrelevant.

For #51654.
Fixes #53846.

Change-Id: I3f3787c67781cac7ce87939c5706cef8db927dd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/417434
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit c006b7a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/417476
@golang golang locked and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@prattmic prattmic

Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
Go1.17.13
Development

No branches or pull requests
3 participants
@toothrot @prattmic @gopherbot