Commit d0d6897f authored by Minoru Chikamune's avatar Minoru Chikamune Committed by Commit Bot

[MBI] Get rid of g_current_agent_group_scheduler_impl global variable

[Target problem]
 - The current code uses g_current_agent_group_scheduler_impl global
   variable.

[What this CL does]
 - Get rid of the g_current_agent_group_scheduler_impl global
   variable, and keep this pointer in MainThreadSchedulerImpl as a
   field.
 - Make it possible to access current AGS from arbitrary places with
   `ThreadScheduler::Current()->GetCurrentAgentGroupScheduler()`.

[Related design doc]
https://docs.google.com/document/d/1y-vHkrD1z2RtyWYwT6rJkSLHClYNjDDpUbTtBU7l95A

Bug: 1105403
Change-Id: Iac332a9400bc20537e5d64ce700d0e5866e7f406
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427945
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Reviewed-by: default avatarTal Pressman <talp@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810234}
parent 4c648a6b
......@@ -8,6 +8,7 @@
#include "base/test/test_mock_time_task_runner.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
#include "third_party/blink/renderer/platform/testing/scoped_scheduler_overrider.h"
......@@ -39,6 +40,9 @@ class MockIdleDeadlineScheduler final : public ThreadScheduler {
PageScheduler::Delegate*) override {
return nullptr;
}
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override {
return nullptr;
}
scoped_refptr<base::SingleThreadTaskRunner> CompositorTaskRunner() override {
return nullptr;
}
......
......@@ -10,6 +10,7 @@
#include "third_party/blink/renderer/bindings/core/v8/v8_idle_request_callback.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_idle_request_options.h"
#include "third_party/blink/renderer/core/testing/null_execution_context.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/test/fake_task_runner.h"
#include "third_party/blink/renderer/platform/testing/scoped_scheduler_overrider.h"
......@@ -64,7 +65,9 @@ class MockScriptedIdleTaskControllerScheduler final : public ThreadScheduler {
std::unique_ptr<RendererPauseHandle> PauseScheduler() override {
return nullptr;
}
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override {
return nullptr;
}
base::TimeTicks MonotonicallyIncreasingVirtualTime() override {
return base::TimeTicks();
}
......
......@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/platform/scheduler/public/dummy_schedulers.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/page_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread.h"
......@@ -183,6 +184,9 @@ class DummyThreadScheduler : public ThreadScheduler {
PageScheduler::Delegate*) override {
return std::make_unique<DummyPageScheduler>();
}
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override {
return nullptr;
}
// ThreadScheduler implementation:
bool ShouldYieldForHighPriorityWork() override { return false; }
......
......@@ -6,6 +6,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
namespace blink {
......@@ -66,6 +67,10 @@ std::unique_ptr<PageScheduler> SimpleThreadScheduler::CreatePageScheduler(
return nullptr;
}
AgentGroupScheduler* SimpleThreadScheduler::GetCurrentAgentGroupScheduler() {
return nullptr;
}
std::unique_ptr<ThreadScheduler::RendererPauseHandle>
SimpleThreadScheduler::PauseScheduler() {
return nullptr;
......
......@@ -8,6 +8,7 @@
#include <memory>
#include "base/macros.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
namespace blink {
......@@ -62,6 +63,9 @@ class SimpleThreadScheduler : public ThreadScheduler {
std::unique_ptr<PageScheduler> CreatePageScheduler(
PageScheduler::Delegate*) override;
// Return nullptr
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override;
// Unsupported. Return nullptr, and it may cause a crash.
std::unique_ptr<RendererPauseHandle> PauseScheduler() override;
......
......@@ -18,21 +18,6 @@ MainThreadTaskQueue::QueueCreationParams DefaultTaskQueueCreationParams(
.SetAgentGroupScheduler(agent_group_scheduler_impl);
}
static AgentGroupSchedulerImpl* g_current_agent_group_scheduler_impl;
// static
AgentGroupSchedulerImpl* AgentGroupSchedulerImpl::GetCurrent() {
DCHECK(WTF::IsMainThread());
return g_current_agent_group_scheduler_impl;
}
// static
void AgentGroupSchedulerImpl::SetCurrent(
AgentGroupSchedulerImpl* agent_group_scheduler_impl) {
DCHECK(WTF::IsMainThread());
g_current_agent_group_scheduler_impl = agent_group_scheduler_impl;
}
AgentGroupSchedulerImpl::AgentGroupSchedulerImpl(
MainThreadSchedulerImpl* main_thread_scheduler)
: default_task_queue_(main_thread_scheduler->NewTaskQueue(
......
......@@ -23,14 +23,12 @@ class MainThreadTaskQueue;
class PLATFORM_EXPORT AgentGroupSchedulerImpl
: public blink::AgentGroupScheduler {
public:
static AgentGroupSchedulerImpl* GetCurrent();
static void SetCurrent(AgentGroupSchedulerImpl*);
explicit AgentGroupSchedulerImpl(
MainThreadSchedulerImpl* main_thread_scheduler);
AgentGroupSchedulerImpl(const AgentGroupSchedulerImpl&) = delete;
AgentGroupSchedulerImpl& operator=(const AgentGroupSchedulerImpl&) = delete;
~AgentGroupSchedulerImpl() override;
scoped_refptr<base::SingleThreadTaskRunner> DefaultTaskRunner() {
scoped_refptr<base::SingleThreadTaskRunner> DefaultTaskRunner() override {
return default_task_runner_;
}
MainThreadSchedulerImpl* GetMainThreadScheduler() {
......
......@@ -2340,6 +2340,25 @@ std::unique_ptr<PageScheduler> MainThreadSchedulerImpl::CreatePageScheduler(
return page_scheduler;
}
AgentGroupScheduler* MainThreadSchedulerImpl::GetCurrentAgentGroupScheduler() {
helper_.CheckOnValidThread();
return current_agent_group_scheduler_;
}
void MainThreadSchedulerImpl::SetCurrentAgentGroupScheduler(
AgentGroupSchedulerImpl* agent_group_scheduler_impl) {
helper_.CheckOnValidThread();
current_agent_group_scheduler_ = agent_group_scheduler_impl;
if (agent_group_scheduler_impl) {
sequence_manager_->SetDefaultTaskRunner(
agent_group_scheduler_impl->DefaultTaskRunner());
} else {
// If there is no proper AgentGroupScheduler, it means that the
// current scheduler is MainThreadScheduler.
sequence_manager_->SetDefaultTaskRunner(helper_.DefaultTaskRunner());
}
}
std::unique_ptr<ThreadScheduler::RendererPauseHandle>
MainThreadSchedulerImpl::PauseScheduler() {
return PauseRenderer();
......@@ -2447,20 +2466,8 @@ void MainThreadSchedulerImpl::OnTaskStarted(
MainThreadTaskQueue* queue,
const base::sequence_manager::Task& task,
const TaskQueue::TaskTiming& task_timing) {
// Switch current active scheduler
{
AgentGroupSchedulerImpl* agent_group_scheduler =
queue ? queue->GetAgentGroupScheduler() : nullptr;
AgentGroupSchedulerImpl::SetCurrent(agent_group_scheduler);
if (agent_group_scheduler) {
sequence_manager_->SetDefaultTaskRunner(
agent_group_scheduler->DefaultTaskRunner());
} else {
// If there is no proper AgentGroupScheduler, it means that the
// current scheduler is MainThreadScheduler.
sequence_manager_->SetDefaultTaskRunner(helper_.DefaultTaskRunner());
}
}
SetCurrentAgentGroupScheduler(queue ? queue->GetAgentGroupScheduler()
: nullptr);
main_thread_only().running_queues.push(queue);
if (main_thread_only().nested_runloop)
......@@ -2522,10 +2529,9 @@ void MainThreadSchedulerImpl::OnTaskCompleted(
find_in_page_budget_pool_controller_->OnTaskCompleted(queue.get(),
task_timing);
// AgentGroupSchedulerImpl::GetCurrent() should return nullptr when
// GetCurrentAgentGroupScheduler() should return nullptr when
// it's running thread global task runners.
AgentGroupSchedulerImpl::SetCurrent(nullptr);
sequence_manager_->SetDefaultTaskRunner(helper_.DefaultTaskRunner());
SetCurrentAgentGroupScheduler(nullptr);
}
void MainThreadSchedulerImpl::RecordTaskUkm(
......
......@@ -224,6 +224,9 @@ class PLATFORM_EXPORT MainThreadSchedulerImpl
AgentGroupSchedulerImpl* EnsureAgentGroupScheduler();
std::unique_ptr<PageScheduler> CreatePageScheduler(
PageScheduler::Delegate*) override;
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override;
void SetCurrentAgentGroupScheduler(
AgentGroupSchedulerImpl* agent_group_scheduler_impl);
std::unique_ptr<ThreadScheduler::RendererPauseHandle> PauseScheduler()
override;
base::TimeTicks MonotonicallyIncreasingVirtualTime() override;
......@@ -1014,6 +1017,7 @@ class PLATFORM_EXPORT MainThreadSchedulerImpl
PollableThreadSafeFlag policy_may_need_update_;
PollableThreadSafeFlag notify_agent_strategy_task_posted_;
WTF::HashSet<AgentGroupSchedulerImpl*> agent_group_schedulers_;
AgentGroupSchedulerImpl* current_agent_group_scheduler_{nullptr};
// TODO(crbug/1113102): tentatively, we hold AgentGroupSchedulerImpl here.
std::unique_ptr<AgentGroupSchedulerImpl> agent_group_scheduler_;
......
......@@ -5,6 +5,7 @@
#ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_SCHEDULER_PUBLIC_AGENT_GROUP_SCHEDULER_H_
#define THIRD_PARTY_BLINK_RENDERER_PLATFORM_SCHEDULER_PUBLIC_AGENT_GROUP_SCHEDULER_H_
#include "base/single_thread_task_runner.h"
#include "third_party/blink/renderer/platform/platform_export.h"
namespace blink {
......@@ -13,6 +14,7 @@ namespace blink {
class PLATFORM_EXPORT AgentGroupScheduler {
public:
virtual ~AgentGroupScheduler() = default;
virtual scoped_refptr<base::SingleThreadTaskRunner> DefaultTaskRunner() = 0;
};
} // namespace blink
......
......@@ -12,6 +12,7 @@
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/common/input/web_input_event_attribution.h"
#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/agent_group_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/page_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread.h"
#include "third_party/blink/renderer/platform/wtf/vector.h"
......@@ -115,6 +116,10 @@ class PLATFORM_EXPORT ThreadScheduler {
virtual std::unique_ptr<PageScheduler> CreatePageScheduler(
PageScheduler::Delegate*) = 0;
// Return the current active AgentGroupScheduler.
// If there is no active AgentGroupScheduler, it returns nullptr.
virtual AgentGroupScheduler* GetCurrentAgentGroupScheduler() = 0;
// Pauses the scheduler. See WebThreadScheduler::PauseRenderer for
// details. May only be called from the main thread.
virtual std::unique_ptr<RendererPauseHandle> PauseScheduler()
......
......@@ -78,6 +78,12 @@ NonMainThreadSchedulerImpl::CreatePageScheduler(
return nullptr;
}
AgentGroupScheduler*
NonMainThreadSchedulerImpl::GetCurrentAgentGroupScheduler() {
NOTREACHED();
return nullptr;
}
std::unique_ptr<NonMainThreadSchedulerImpl::RendererPauseHandle>
NonMainThreadSchedulerImpl::PauseScheduler() {
return nullptr;
......
......@@ -21,6 +21,7 @@
#include "third_party/blink/renderer/platform/scheduler/worker/non_main_thread_task_queue.h"
namespace blink {
class AgentGroupScheduler;
namespace scheduler {
class WorkerSchedulerProxy;
......@@ -72,6 +73,7 @@ class PLATFORM_EXPORT NonMainThreadSchedulerImpl : public ThreadSchedulerImpl {
std::unique_ptr<PageScheduler> CreatePageScheduler(
PageScheduler::Delegate*) override;
AgentGroupScheduler* GetCurrentAgentGroupScheduler() override;
std::unique_ptr<RendererPauseHandle> PauseScheduler() override
WARN_UNUSED_RESULT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment