Introduce SlimSchedulerStateMachine to prevent re-entrancy [chromium/src : main]
0 views
Skip to first unread message
Jonathan Ross (Gerrit)
Mar 11, 2026, 2:06:17 PMMar 11
to Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Stacy Gaikovaia
Jonathan Ross added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Jonathan Ross . resolved
Here's the next variance of scheduler. Could you PTAL? Thanks!
Related details
Attention is currently required from:
- Stacy Gaikovaia
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stacy Gaikovaia (Gerrit)
Mar 11, 2026, 2:31:54 PMMar 11
to Jonathan Ross, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Jonathan Ross
Stacy Gaikovaia added 3 comments![]( "Open in Gerrit")
File cc/scheduler/scheduler_state_machine.h
Line 429, Patchset 4 (Latest):
virtual bool ShouldBlockBeginMainFrameWhenIdle() const;Stacy Gaikovaia . unresolved
Maybe we should rename the function in a follow-up cl 😊 So that each cl does one small thing
File cc/scheduler/slim_scheduler_state_machine.h
Line 25, Patchset 4 (Latest):
// - Allows beginning `BeginMainFrame` while idle to begin preparing the next
// frameStacy Gaikovaia . resolved
We should follow-up on how this could work when cc scheduler is no longer responsible for sending the BMF signal.
File cc/scheduler/slim_scheduler_state_machine_unittest.cc
File-level comment, Patchset 4 (Parent):
Stacy Gaikovaia . unresolved
Maybe we can have a second unit test that checks things like ACTION::COMMIT or ACTION::SEND_BEGIN_MAIN_FRAME or ACTION::ACTIVATE_SYNC_TREE
Related details
Attention is currently required from:
- Jonathan Ross
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 11, 2026, 4:21:15 PMMar 11
to Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Stacy Gaikovaia
Jonathan Ross added 2 comments![]( "Open in Gerrit")
File cc/scheduler/scheduler_state_machine.h
Line 429, Patchset 4 (Latest):
virtual bool ShouldBlockBeginMainFrameWhenIdle() const;Stacy Gaikovaia . resolved
Maybe we should rename the function in a follow-up cl 😊 So that each cl does one small thing
Jonathan Ross
https://chromium-review.git.corp.google.com/c/chromium/src/+/7657486 will rebase once that's landed
File cc/scheduler/slim_scheduler_state_machine_unittest.cc
Stacy Gaikovaia . resolved
Maybe we can have a second unit test that checks things like ACTION::COMMIT or ACTION::SEND_BEGIN_MAIN_FRAME or ACTION::ACTIVATE_SYNC_TREE
Attention is currently required from:
- Stacy Gaikovaia
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stacy Gaikovaia (Gerrit)
Mar 11, 2026, 4:23:26 PMMar 11
to Jonathan Ross, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Jonathan Ross
Stacy Gaikovaia voted and added 1 comment![]( "Open in Gerrit")
Votes added by Stacy Gaikovaia
| Code-Review | +1 |
1 comment
Patchset-level comments
Stacy Gaikovaia . resolved
Every one of our problems can be solved with a new cc scheduler subclass.
Related details
Attention is currently required from:
- Jonathan Ross
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stacy Gaikovaia (Gerrit)
Mar 11, 2026, 4:24:40 PMMar 11
to Jonathan Ross, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Jonathan Ross
Stacy Gaikovaia added 1 comment![]( "Open in Gerrit")
File cc/scheduler/slim_scheduler_state_machine_unittest.cc
Stacy Gaikovaia . unresolved
Jonathan RossMaybe we can have a second unit test that checks things like ACTION::COMMIT or ACTION::SEND_BEGIN_MAIN_FRAME or ACTION::ACTIVATE_SYNC_TREE
Done
Stacy Gaikovaia
I don't see it!
Related details
Attention is currently required from:
- Jonathan Ross
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 11, 2026, 4:48:42 PMMar 11
to Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Jonathan Ross added 1 comment![]( "Open in Gerrit")
File cc/scheduler/slim_scheduler_state_machine_unittest.cc
Stacy Gaikovaia . resolved
Jonathan RossMaybe we can have a second unit test that checks things like ACTION::COMMIT or ACTION::SEND_BEGIN_MAIN_FRAME or ACTION::ACTIVATE_SYNC_TREE
Stacy GaikovaiaDone
I don't see it!
Jonathan Ross
Apaprently the local tree had a rebase warning that prevented the upload. My bad
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 17, 2026, 12:14:10 PMMar 17
to Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Jonathan Ross voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 17, 2026, 4:03:50 PMMar 17
to Victor Miura, Chromium LUCI CQ, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Stacy Gaikovaia and Victor Miura
Jonathan Ross added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 6 (Latest):
Jonathan Ross . resolved
+vmiura@ for thoughts on this variant of the scheduler. To prevent re-entrancy and to restrict to immediate/waitforscrolls
Related details
Attention is currently required from:
- Stacy Gaikovaia
- Victor Miura
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Miura (Gerrit)
Mar 19, 2026, 5:58:33 PM (14 days ago) Mar 19
to Jonathan Ross, Chromium LUCI CQ, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Jonathan Ross
Victor Miura added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Victor Miura . resolved
Sorry for slow review. Will take a look by tomorrow.
Related details
Attention is currently required from:
- Jonathan Ross
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 26, 2026, 10:20:36 AM (7 days ago) Mar 26
to Victor Miura, Chromium LUCI CQ, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Victor Miura
Jonathan Ross added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Victor Miura . resolved
Sorry for slow review. Will take a look by tomorrow.
Attention is currently required from:
- Victor Miura
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Victor Miura (Gerrit)
Mar 30, 2026, 11:21:43 AM (3 days ago) Mar 30
to Jonathan Ross, Chromium LUCI CQ, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Attention needed from Jonathan Ross
Victor Miura voted and added 2 comments![]( "Open in Gerrit")
Votes added by Victor Miura
| Code-Review | +1 |
2 comments
Patchset-level comments
Victor Miura . resolved
LGTM, thanks
Commit Message
Line 9, Patchset 6 (Latest):
This state machine's supports:Victor Miura . unresolved
nit: `This state machine supports` ?
Related details
Attention is currently required from:
- Jonathan Ross
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jonathan Ross (Gerrit)
Mar 30, 2026, 11:24:20 AM (3 days ago) Mar 30
to Victor Miura, Chromium LUCI CQ, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Jonathan Ross voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jonathan Ross
| Commit-Queue | +2 |
nit: `This state machine supports` ?
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Mar 30, 2026, 12:48:37 PM (3 days ago) Mar 30
to Jonathan Ross, Victor Miura, Stacy Gaikovaia, chrom...@appspot.gserviceaccount.com, chromium...@chromium.org, cc-...@chromium.org, schedule...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
6 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Introduce SlimSchedulerStateMachine to prevent re-entrancy
This state machine supports:
- Prevents re-entrancy during a VSync Interval. We will no longer repeat an `Action`
- Removed BeginImplFrameDeadlineMode::REGULAR and LATE. As these modes lead to the next VSync being IMMEDIATE. This can result in two frame submissions within less than a VSync interval. This leads to dropped frames or exhaustion of frame buffers, which pauses rendering.
- Restricts different `Action` to different `BeginImplFrameState`. So that we only attempt to Commit and Activate after submitting the current frame, when we are IDLE.
- Allows beginning `BeginMainFrame` while idle to begin preparing the next frame
- Allows `PrepareTiles` while idle, so that this doesn't slow down frame submission.
Bug: 466262971
Change-Id: If0329ac33fa5d6d7fd200bdad4c22131c47591a3
Commit-Queue: Jonathan Ross <jon...@chromium.org>
Reviewed-by: Victor Miura <vmi...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1607168}
Files:
- M cc/BUILD.gn
- M cc/base/features.cc
- M cc/base/features.h
- M cc/scheduler/scheduler.cc
- M cc/scheduler/scheduler_settings.h
- M cc/scheduler/scheduler_state_machine.h
- A cc/scheduler/slim_scheduler_state_machine.cc
- A cc/scheduler/slim_scheduler_state_machine.h
- A cc/scheduler/slim_scheduler_state_machine_unittest.cc
- M cc/trees/layer_tree_settings.cc
Change size: L
Delta: 10 files changed, 282 insertions(+), 2 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Victor Miura
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages