Commit 88c337cc authored by Hailey Wang's avatar Hailey Wang Committed by Commit Bot

[PM] Utilize MaxSimultaneousLoad threshold

Add usage of MaxSimultaneousLoad threshold in
BackgroundTabLoadingPolicy's loading logic.

Original CL: https://chromium-review.googlesource.com/c/chromium/src/+/2122479
Reland due to test failing because of dependencies on system core
number. Removed these dependencies in unit tests.

Bug: 1059341
Change-Id: I20fd569aa13c2912ba8f6366bf5d3916f72e8f58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134190Reviewed-by: default avatarSébastien Marchand <sebmarchand@chromium.org>
Reviewed-by: default avatarFrançois Doray <fdoray@chromium.org>
Commit-Queue: Hailey Wang <haileywang@google.com>
Cr-Commit-Position: refs/heads/master@{#756267}
parent cb26d351
......@@ -64,7 +64,7 @@ BackgroundTabLoadingPolicy::BackgroundTabLoadingPolicy()
: page_loader_(std::make_unique<mechanism::PageLoader>()) {
DCHECK(!g_background_tab_loading_policy);
g_background_tab_loading_policy = this;
simultaneous_tab_loads_ = CalculateMaxSimultaneousTabLoads(
max_simultaneous_tab_loads_ = CalculateMaxSimultaneousTabLoads(
kMinSimultaneousTabLoads, kMaxSimultaneousTabLoads,
kCoresPerSimultaneousTabLoad, base::SysInfo::NumberOfProcessors());
}
......@@ -86,6 +86,9 @@ void BackgroundTabLoadingPolicy::OnIsLoadingChanged(const PageNode* page_node) {
if (!page_node->IsLoading()) {
// Once the PageNode finishes loading, stop tracking it within this policy.
RemovePageNode(page_node);
// Since there is a free loading slot, load more tab if needed.
MaybeLoadSomeTabs();
return;
}
// The PageNode started loading, either because of this policy or because of
......@@ -105,6 +108,10 @@ void BackgroundTabLoadingPolicy::OnIsLoadingChanged(const PageNode* page_node) {
void BackgroundTabLoadingPolicy::OnBeforePageNodeRemoved(
const PageNode* page_node) {
RemovePageNode(page_node);
// There may be free loading slots, check and load more tabs if that's the
// case.
MaybeLoadSomeTabs();
}
void BackgroundTabLoadingPolicy::ScheduleLoadForRestoredTabs(
......@@ -115,8 +122,8 @@ void BackgroundTabLoadingPolicy::ScheduleLoadForRestoredTabs(
page_nodes_to_load_.push_back(page_node);
DCHECK(
TabPropertiesDecorator::Data::FromPageNode(page_node)->IsInTabStrip());
InitiateLoad(page_node);
}
MaybeLoadSomeTabs();
}
void BackgroundTabLoadingPolicy::SetMockLoaderForTesting(
......@@ -124,11 +131,16 @@ void BackgroundTabLoadingPolicy::SetMockLoaderForTesting(
page_loader_ = std::move(loader);
}
void BackgroundTabLoadingPolicy::SetMaxSimultaneousLoadsForTesting(
size_t loading_slots) {
max_simultaneous_tab_loads_ = loading_slots;
}
BackgroundTabLoadingPolicy* BackgroundTabLoadingPolicy::GetInstance() {
return g_background_tab_loading_policy;
}
void BackgroundTabLoadingPolicy::InitiateLoad(PageNode* page_node) {
void BackgroundTabLoadingPolicy::InitiateLoad(const PageNode* page_node) {
// Mark |page_node| as load initiated. Ensure that InitiateLoad is only called
// for a PageNode that is tracked by the policy.
size_t num_removed = base::Erase(page_nodes_to_load_, page_node);
......@@ -145,6 +157,44 @@ void BackgroundTabLoadingPolicy::RemovePageNode(const PageNode* page_node) {
base::Erase(page_nodes_loading_, page_node);
}
void BackgroundTabLoadingPolicy::MaybeLoadSomeTabs() {
// Continue to load tabs while possible. This is in a loop with a
// recalculation of GetMaxNewTabLoads() as reentrancy can cause conditions
// to change as each tab load is initiated.
while (GetMaxNewTabLoads() > 0)
LoadNextTab();
}
size_t BackgroundTabLoadingPolicy::GetMaxNewTabLoads() const {
// This takes into account all tabs currently loading across the browser,
// including ones that BackgroundTabLoadingPolicy isn't explicitly managing.
// This ensures that BackgroundTabLoadingPolicy respects user interaction
// first and foremost. There's a small race between when we initiated loading
// and when PageNodeObserver notifies us that it has actually started, so we
// also make use of |page_nodes_initiated_| to track these.
size_t loading_tab_count =
page_nodes_load_initiated_.size() + page_nodes_loading_.size();
// Determine the number of free loading slots available.
size_t page_nodes_to_load = 0;
if (loading_tab_count < max_simultaneous_tab_loads_)
page_nodes_to_load = max_simultaneous_tab_loads_ - loading_tab_count;
// Cap the number of loads by the actual number of tabs remaining.
page_nodes_to_load = std::min(page_nodes_to_load, page_nodes_to_load_.size());
return page_nodes_to_load;
}
void BackgroundTabLoadingPolicy::LoadNextTab() {
DCHECK(!page_nodes_to_load_.empty());
// Find the next PageNode to load.
const PageNode* page_node = page_nodes_to_load_.front();
InitiateLoad(page_node);
}
} // namespace policies
} // namespace performance_manager
......@@ -42,6 +42,7 @@ class BackgroundTabLoadingPolicy : public GraphOwned,
void ScheduleLoadForRestoredTabs(std::vector<PageNode*> page_nodes);
void SetMockLoaderForTesting(std::unique_ptr<mechanism::PageLoader> loader);
void SetMaxSimultaneousLoadsForTesting(size_t loading_slots);
// Returns the instance of BackgroundTabLoadingPolicy within the graph.
static BackgroundTabLoadingPolicy* GetInstance();
......@@ -49,12 +50,25 @@ class BackgroundTabLoadingPolicy : public GraphOwned,
private:
// Move the PageNode from |page_nodes_to_load_| to
// |page_nodes_load_initiated_| and make the call to load the PageNode.
void InitiateLoad(PageNode* page_node);
void InitiateLoad(const PageNode* page_node);
// Removes the PageNode from all the sets of PageNodes that the policy is
// tracking.
void RemovePageNode(const PageNode* page_node);
// Initiates the load of enough tabs to fill all loading slots. No-ops if all
// loading slots are occupied.
void MaybeLoadSomeTabs();
// Determines the number of tab loads that can be started at the moment to
// avoid exceeding the number of loading slots.
size_t GetMaxNewTabLoads() const;
// Loads the next tab. This should only be called if there is a next tab to
// load. This will always start loading a next tab even if the number of
// simultaneously loading tabs is exceeded.
void LoadNextTab();
// The mechanism used to load the pages.
std::unique_ptr<performance_manager::mechanism::PageLoader> page_loader_;
......@@ -72,7 +86,7 @@ class BackgroundTabLoadingPolicy : public GraphOwned,
// The number of simultaneous tab loads that are permitted by policy. This
// is computed based on the number of cores on the machine.
size_t simultaneous_tab_loads_;
size_t max_simultaneous_tab_loads_;
};
} // namespace policies
......
......@@ -49,6 +49,10 @@ class BackgroundTabLoadingPolicyTest : public GraphTestHarness {
auto mock_loader = std::make_unique<MockPageLoader>();
mock_loader_ = mock_loader.get();
policy_->SetMockLoaderForTesting(std::move(mock_loader));
// Set a value explicitly for MaxSimultaneousLoad threshold to avoid a
// dependency on the number of cores of the machine on which the test runs.
policy_->SetMaxSimultaneousLoadsForTesting(4);
}
void TearDown() override { graph()->TakeFromGraph(policy_); }
......@@ -82,6 +86,45 @@ TEST_F(BackgroundTabLoadingPolicyTest, ScheduleLoadForRestoredTabs) {
policy()->ScheduleLoadForRestoredTabs(raw_page_nodes);
}
TEST_F(BackgroundTabLoadingPolicyTest, AllLoadingSlotsUsed) {
// Create 4 PageNode to restore.
std::vector<
performance_manager::TestNodeWrapper<performance_manager::PageNodeImpl>>
page_nodes;
std::vector<PageNode*> raw_page_nodes;
// Create vector of PageNode to restore.
for (int i = 0; i < 4; i++) {
page_nodes.push_back(CreateNode<performance_manager::PageNodeImpl>());
raw_page_nodes.push_back(page_nodes.back().get());
// Set |is_tab| property as this is a requirement to pass the PageNode to
// ScheduleLoadForRestoredTabs().
TabPropertiesDecorator::SetIsTabForTesting(raw_page_nodes.back(), true);
}
PageNodeImpl* page_node_impl = page_nodes[0].get();
EXPECT_CALL(*loader(), LoadPageNode(raw_page_nodes[0]));
EXPECT_CALL(*loader(), LoadPageNode(raw_page_nodes[1]));
// Use 2 loading slots, which means only 2 of the PageNodes should immediately
// be scheduled to load.
policy()->SetMaxSimultaneousLoadsForTesting(2);
policy()->ScheduleLoadForRestoredTabs(raw_page_nodes);
testing::Mock::VerifyAndClear(loader());
// Simulate load start of a PageNode that initiated load.
page_node_impl->SetIsLoading(true);
// The policy should allow one more PageNode to load after a PageNode finishes
// loading.
EXPECT_CALL(*loader(), LoadPageNode(raw_page_nodes[2]));
// Simulate load finish of a PageNode.
page_node_impl->SetIsLoading(false);
}
} // namespace policies
} // namespace performance_manager
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