New issue
Advanced search Search tips

Issue 862589 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

virtual/threaded/fast/idle-callback/ tests fail in HeapIncrementalMarkingStress mode

Project Member Reported by keishi@chromium.org, Jul 11

Issue description

Following tests fail regularly on the HeapIncrementalMarkingStress bot
virtual/threaded/fast/idle-callback/basic.html
virtual/threaded/fast/idle-callback/timeout.html	
virtual/threaded/fast/idle-callback/long_idle_periods.html

The tests timeout because the idle callback is never called.

This seems to happen because IncrementalMarkingFinalize tries to register a idle task for lazy sweeping, which calls MainThreadSchedulerImpl::OnPendingTasksChanged(). But it gets called too early, before the PageSchedulers are added to main_thread_only().page_schedulers. This means that cc::Scheduler::BeginMainFrameNotExpectedUntil() is not called but since main_thread_only().compositor_will_send_main_frame_not_expected remembers that we called MainThreadSchedulerImpl::DispatchRequestBeginMainFrameNotExpected, MainThreadSchedulerImpl will wait forever for the pending idle tasks to be executed.

I need to take a closer look to see where we will fix.
 
If we don't have schedulers, we wont get any tasks properly run, right?

Maybe just do a whatever we would have done (lazy sweeping in this case) eagerly?

Is this also a problem for the regular GC that tries to schedule a lazy sweeping task?
I think the problem is that we are posting an idle task too early, before even one PageScheduler is ready. My current gut feeling is that we shouldn't be setting main_thread_only().compositor_will_send_main_frame_not_expected when main_thread_only().page_schedulers.size() == 0.
main_thread_only().compositor_will_send_main_frame_not_expected is caching the fact that we set a flag on the compositor. But since the PageScheduler doesn't exist the flag is not actually set.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1033f682f37d7d5a4f5668275656804c7a577f2

commit b1033f682f37d7d5a4f5668275656804c7a577f2
Author: Keishi Hattori <keishi@chromium.org>
Date: Wed Jul 18 04:22:39 2018

compositor_will_send_main_frame_not_expected flag should not be set when no PageSchedulers exist

When we set compositor_will_send_main_frame_not_expected flag when no PageSchedulers exist, MainThreadSchedulerImpl::BeginMainFrameNotExpectedUntil() never gets called. This means that StartIdlePeriod rarely gets called and the idle tasks may not be executed for a long time.

After this CL, compositor_will_send_main_frame_not_expected will not be set when no PageSchedulers exist, so MainThreadSchedulerImpl will try calling RequestBeginMainFrameNotExpected next time.

Bug: 862589
Change-Id: I06f9c813df6cc7270269d9ffb3a01f24b9c911ce
Reviewed-on: https://chromium-review.googlesource.com/1134630
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575938}
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/core/page/page.cc
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/core/page/page.h
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl_unittest.cc
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/main_thread/page_scheduler_impl.cc
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/main_thread/page_scheduler_impl.h
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/public/page_scheduler.h
[modify] https://crrev.com/b1033f682f37d7d5a4f5668275656804c7a577f2/third_party/blink/renderer/platform/scheduler/test/fake_page_scheduler.h

At least for virtual/threaded/fast/idle-callback/long_idle_periods.html, the flakiness is still very prominent, see https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=virtual%2Fthreaded%2Ffast%2Fidle-callback%2Flong_idle_periods.html&testType=webkit_layout_tests

As this test is timing out approximately in every 5th build, I am going to disable it for now in https://crrev.com/c/1180963 since marking as SLOW was already done before.

A recent timeout:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%281%29/82902
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bd6c7f9fb4c56849ce311251c2ccf899029a7cab

commit bd6c7f9fb4c56849ce311251c2ccf899029a7cab
Author: Friedrich Horschig <fhorschig@chromium.org>
Date: Mon Aug 20 10:36:38 2018

[Flaky] Mark long_idle_periods.html as prone to timing out

This test is known to be flaky and a change related to it didn't change
that (see linked bug).

TBR=keishi@chromium.org

Bug: 862589
Change-Id: I3c64042bb3d36c9515d2b1b302d13b85fbbc8034
Reviewed-on: https://chromium-review.googlesource.com/1180963
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584398}
[modify] https://crrev.com/bd6c7f9fb4c56849ce311251c2ccf899029a7cab/third_party/WebKit/LayoutTests/TestExpectations

Sign in to add a comment